Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🏃 [e2e] clusterctl e2e config #2819

Merged

Conversation

fabriziopandini
Copy link
Member

What this PR does / why we need it:
This PR extends the e2e framework by adding support for a configuration file designed for e2e tests based on clusterctl/ready to run on different infrastructure providers.

The PR includes also a utility for creating a local clusterctl repository that contains the providers defined in the e2e configuration and a clusterctl config file to make clusterctl to use this repository.

Most notably:

  • The format support list of images to be added to the kind image (same a current config)
  • The list of components of the current config is replaced by a list of providers, and it is not required to add cert-manager anymore
  • For each provider
    • it is possible to specify more than one version to be included in the test (for testing provider upgrades)
    • it is possible to specify additional files, like e.g. cluster-templates to be included in the test
    • it is not required to specify waiter anymore
  • It is possible to specify intervals for wait loops (they may vary across providers)

Which issue(s) this PR fixes:
xref #2753
xref #2637
xref #2636

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Mar 30, 2020
@k8s-ci-robot k8s-ci-robot requested review from detiber and ncdc March 30, 2020 12:36
@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Mar 30, 2020
@fabriziopandini
Copy link
Member Author

/assign @vincepri
/assign @sedefsavas
/cc @wfernandes
/cc @gab-satchi

Copy link
Member

@vincepri vincepri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/approve
/assign @sedefsavas
/milestone v0.3.4

@k8s-ci-robot k8s-ci-robot added this to the v0.3.4 milestone Mar 30, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fabriziopandini, vincepri

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 30, 2020

"github.com/pkg/errors"
clusterctlv1 "sigs.k8s.io/cluster-api/cmd/clusterctl/api/v1alpha3"
clusterctlconfig "sigs.k8s.io/cluster-api/cmd/clusterctl/client/config"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May want to rename this as there is a clusterctlConfig type in test/framework/clusterctl/clusterctl_config.go

Copy link
Member Author

@fabriziopandini fabriziopandini Apr 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is an import from the clusterctl library, so the name makes sense for me because it uses the same root of e.g. clusterctlv1. Instead IMO everything under test/framework/clusterctl should be reported as testclusterctl, but this is not the case.
But if you have a better suggestion happy to change this

@wfernandes
Copy link
Contributor

reviewing

Values map[string]interface{}
}

// providerConfig mirrors the clusterctl config.Provider interface and allows serialization of the corresponding info into a clusterctl config file.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since it should mirror the interface should we do something like var _ config.Provider = &providerConfig{} as a check.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should not work, because the interface implements methods, while here we are required to implement Field to get serialization to work

test/framework/clusterctl/e2e_config.go Show resolved Hide resolved
test/framework/clusterctl/e2e_config.go Show resolved Hide resolved
@fabriziopandini
Copy link
Member Author

@wfernandes @sedefsavas thanks for the feedback!
Comment addressed/answer provided

@sedefsavas
Copy link

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 6, 2020
@k8s-ci-robot k8s-ci-robot merged commit 0c7d6e0 into kubernetes-sigs:master Apr 6, 2020
@fabriziopandini fabriziopandini deleted the add-clusterctl-e2e-config branch April 6, 2020 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants