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

🐛 make manifest to use cert-manager.io/v1alpha2 #4411

Conversation

fabriziopandini
Copy link
Member

@fabriziopandini fabriziopandini commented Mar 30, 2021

What this PR does / why we need it:
#4225 made clustectl to deploy cert manager v1.1.0. and updated manifests generation in order to generate Certificate objects using cert-manager.io/v1 apiVersion.

Unfortunately this is creating a problem for people using old versions of clusterctl for doing clusterctl init because e.g.:

  • clusterctl init with clusterctl v0.3.14 installs cert manager v0.16.1
  • clusterctl init then installs the latest version of providers, that now is v0.3.15 which creates cert-manager.io/v1 objects.

This makes init to fail with older version of clusterctl.

This PR fixes this problem by changing our manifests in order to generate Certificate objects using cert-manager.io/v1alpha2 apiVersion, which can be processed either by v0.16.1 and v1.1.0 version of cert manager

Note
As a temporary workaround it is possible to pin version of the installed providers

clusterctl init \
	--core cluster-api:v0.3.14 \
	--bootstrap kubeadm:v0.3.14 \
	--control-plane kubeadm:v0.3.14 \
	--infrastructure vsphere:v0.3.14

@k8s-ci-robot k8s-ci-robot added this to the v0.3 milestone Mar 30, 2021
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Mar 30, 2021
@k8s-ci-robot k8s-ci-robot requested review from benmoss and detiber March 30, 2021 18:58
@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Mar 30, 2021
@CecileRobertMichon
Copy link
Contributor

CecileRobertMichon commented Mar 30, 2021

I see this PR is targeting release-0.3, is the plan to keep cert-manager.io/v1 in 0.4?

@fabriziopandini
Copy link
Member Author

I see this PR is targeting release-0.3, is the plan to keep cert-manager.io/v1 in 0.4?

given that it is required to use clusterctl v0.4.0 to create v1alpha4 clusters, it is fine to keep cert-manager.io/v1 in 0.4.
the problem is only on the v0.3 branch because people can use old version of clusterctl to create clusters with CAPI >= 0.3.15

@sbueringer
Copy link
Member

/lgtm
seems reasonable to me

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 31, 2021
@detiber
Copy link
Member

detiber commented Mar 31, 2021

/approve
/hold
hold for @CecileRobertMichon to have a chance to see if the response was sufficient.

@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Mar 31, 2021
@vincepri
Copy link
Member

vincepri commented Mar 31, 2021

Given that the other PR was marked as ⚠️, should we downgrade here, or force the use of the newer clusterctl version instead?

IIRC, clusterctl should check if there is a new version of the CLI available, is that not the case?

I'd mostly like to understand if we need to keep full compatibility between the manifests going forward, I'm ok either way.

@fabriziopandini
Copy link
Member Author

fabriziopandini commented Mar 31, 2021

I think we should aim to full compatibility between the manifests going forward (within a minor), and in this case this can be achieved with a small change.
While we are alpha, I guess we can break this promise if necessary e.g. due to security patches, but in that case we should make a better job in informing users...

@vincepri
Copy link
Member

/approve
/lgtm

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: detiber, 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

@CecileRobertMichon
Copy link
Contributor

/lgtm
/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 31, 2021
@k8s-ci-robot k8s-ci-robot merged commit e74836f into kubernetes-sigs:release-0.3 Mar 31, 2021
@fabriziopandini fabriziopandini deleted the make-manifest-use-cert-manager.io/v1alpha2 branch April 6, 2021 10:08
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/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants