Skip to content

Update instructions for upgrading cert-manager resources and CRDs#793

Merged
jetstack-bot merged 2 commits intocert-manager:masterfrom
irbekrm:update_upgrade_instructions
Jan 12, 2022
Merged

Update instructions for upgrading cert-manager resources and CRDs#793
jetstack-bot merged 2 commits intocert-manager:masterfrom
irbekrm:update_upgrade_instructions

Conversation

@irbekrm
Copy link
Contributor

@irbekrm irbekrm commented Jan 4, 2022

Update the instructions how to ensure that cert-manager resources are stored in etcd at v1 and that the deprecated API versions are not recorded on CRD status fields to refer to the new cmctl upgrade migrate-api-version command

Testing the upgrade

  1. Create a kind cluster with Kubernetes <= 1.21
  2. helm install cert-manager jetstack/cert-manager -ncert-manager --create-namespace --set installCRDs=true --version v0.16
  3. Create some legacy resources such these:
apiVersion: cert-manager.io/v1alpha2
kind: Issuer
metadata:
  name: selfsigned-issuer-legacy
spec:
  selfSigned: {}
---
apiVersion: cert-manager.io/v1alpha2
kind: Certificate
metadata:
  name: test-old
spec:
  secretName: test-old
  dnsNames:
  - foo.bar
  issuerRef:
    name: selfsigned-issuer-legacy
    kind: Issuer
    group: cert-manager.io

These will be stored in etcd at v1beta1 which is the preferred storage version for cert-manager v0.16

  1. Upgrade to cert-manager v1.6 (or any other cert-manager version that is v1 or higher)
    helm upgrade cert-manager jetstack/cert-manager -ncert-manager --create-namespace --set installCRDs=true --version v1.6.1
    8.Run cmctl upgrade migrate-api-version
  2. Upgrade to cert-manager v1.7.0-alpha.0
  3. Verify that the upgrade works and you can still retrieve all cert-manager resources

Signed-off-by: irbekrm irbekrm@gmail.com

/kind cleanup

@jetstack-bot jetstack-bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 4, 2022
@jetstack-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: irbekrm

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

The pull request process is described here

Details 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

@jetstack-bot jetstack-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 4, 2022
@netlify
Copy link

netlify bot commented Jan 4, 2022

✔️ Deploy Preview for cert-manager-website ready!

🔨 Explore the source changes: 663378c

🔍 Inspect the deploy log: https://app.netlify.com/sites/cert-manager-website/deploys/61dddce188a8c000075b5088

😎 Browse the preview: https://deploy-preview-793--cert-manager-website.netlify.app/docs/installation/upgrading/remove-deprecated-apis

@irbekrm irbekrm force-pushed the update_upgrade_instructions branch 7 times, most recently from 2cd31c3 to 645a335 Compare January 4, 2022 15:32
@irbekrm irbekrm force-pushed the update_upgrade_instructions branch from 645a335 to 3fbe4bb Compare January 4, 2022 15:43
@irbekrm irbekrm changed the title WIP Update instructions for upgrading cert-manager resources and CRDs Update instructions for upgrading cert-manager resources and CRDs Jan 4, 2022
@jetstack-bot jetstack-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 4, 2022
@irbekrm
Copy link
Contributor Author

irbekrm commented Jan 4, 2022

I've tried to add a wording that suggests that these instructions should be applied with caution and also some scripts to verify that things have been upgraded, but it does feel like this is now a bit verbose- happy to remove some of the verbosity or scripts if suggested :)

@irbekrm irbekrm changed the title Update instructions for upgrading cert-manager resources and CRDs WIP Update instructions for upgrading cert-manager resources and CRDs Jan 4, 2022
@jetstack-bot jetstack-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 4, 2022
@irbekrm
Copy link
Contributor Author

irbekrm commented Jan 4, 2022

/hold

some related conversation here https://kubernetes.slack.com/archives/CDEQJ0Q8M/p1641311819078800

We might end up putting all those steps into a tool

@jetstack-bot jetstack-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 4, 2022
@wallrj
Copy link
Member

wallrj commented Jan 7, 2022

The cmctl tool has now merged, so we should reword this PR to explain how and when to use the tool

@irbekrm irbekrm force-pushed the update_upgrade_instructions branch from 3fbe4bb to b8d1b0a Compare January 11, 2022 13:15
@jetstack-bot jetstack-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 11, 2022
@irbekrm irbekrm force-pushed the update_upgrade_instructions branch 2 times, most recently from f757fd7 to 1472747 Compare January 11, 2022 17:41
@jetstack-bot jetstack-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 11, 2022
Signed-off-by: irbekrm <irbekrm@gmail.com>
@irbekrm irbekrm force-pushed the update_upgrade_instructions branch from 1472747 to ec8813b Compare January 11, 2022 18:40
@jetstack-bot jetstack-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 11, 2022
Copy link
Member

@maelvls maelvls left a comment

Choose a reason for hiding this comment

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

I have tested the instructions performed:

  • one upgrade from v1.6.1 to v1.7.0-alpha.0,
  • one upgrade from v1.5.3 to v1.6.1.

Both went smoothly.

That said, could we give some indication to people who use a GitOps-like flow? The step (4) seems to indicate the command has to be run locally, which isn't possible for people using a GitOps flow (cc @jsoref)

- `acme.cert-manager.io/v1alpha3`
- `acme.cert-manager.io/v1beta1`

These APIs are no longer served in cert-manager `v1.6` and are fully removed in cert-manager `v1.7`. If you have a cert-manager installation that is using or has previously used these deprecated APIs you might need to upgrade your cert-manager custom resources and CRDs. This should be done before upgrading to cert-manager `v1.6` or later.
Copy link
Member

Choose a reason for hiding this comment

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

nit: Terminology uses the leading v to refer to cert-manager git tags (we also call these "patch releases" I think). I think by v1.6 you mean "release branch" which may be instead written "1.6" 😇 (I know I know I am way too picky)

Suggested change
These APIs are no longer served in cert-manager `v1.6` and are fully removed in cert-manager `v1.7`. If you have a cert-manager installation that is using or has previously used these deprecated APIs you might need to upgrade your cert-manager custom resources and CRDs. This should be done before upgrading to cert-manager `v1.6` or later.
These APIs are no longer served in cert-manager 1.6 and are fully removed in cert-manager 1.7. If you have a cert-manager installation that is using or has previously used these deprecated APIs you might need to upgrade your cert-manager custom resources and CRDs. This should be done before upgrading to cert-manager 1.6 or later.


3. Make sure that any cert-manager custom resource manifests that refer to the deprecated APIs are updated to use the `cert-manager.io/v1` API and re-applied. You can use the [cmctl convert command](https://cert-manager.io/docs/usage/cmctl/#convert)to convert manifests.

4. Run [cmctl upgrade migrate-api-version command](https://cert-manager.io/docs/usage/cmctl/#migrate-api-version) to migrate any resources that might be stored in `etcd` at the deprecated API versions and to patch the cert-manager CRDs.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
4. Run [cmctl upgrade migrate-api-version command](https://cert-manager.io/docs/usage/cmctl/#migrate-api-version) to migrate any resources that might be stored in `etcd` at the deprecated API versions and to patch the cert-manager CRDs.
4. Run the command [`cmctl upgrade migrate-api-version`](https://cert-manager.io/docs/usage/cmctl/#migrate-api-version) to migrate any resources that might be stored in `etcd` at the deprecated API versions and to patch the cert-manager CRDs.

Copy link
Member

Choose a reason for hiding this comment

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

We should also propose or suggest a GitOps way to run this command. For example, we could suggest to create a Job and mention that an image of cmctl exists. What do you think @jsoref?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Job will not be able to act as a gate for upgrade - then we have to tell folks to check the job status etc. I think that running the tool from command line by default is probably safer and do it from a script in case of many clusters

Copy link
Member

@maelvls maelvls Jan 12, 2022

Choose a reason for hiding this comment

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

Ah right yes, that's a good point. I don't really know how people manage this, since sometimes the only possibility is to commit to a git repository (as opposed to manually running kubectl).

I pinged people on #argo-cd (CNCF slack) just to make sure I understand what people think about this upgrade path.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's a good point, I imagine there is some approach for these kind of situations. I suppose in this case, the simplest solution would be to commit a Job that runs the command and check that it succeeded before upgrading.. but there might be a better solution

@irbekrm
Copy link
Contributor Author

irbekrm commented Jan 11, 2022

That said, could we give some indication to people who use a GitOps-like flow?

I think they should just run it from a script then, unless they use helm or something else that has a concept of pre-install hook.

@jetstack-bot jetstack-bot added dco-signoff: no Indicates that at least one commit in this pull request is missing the DCO sign-off message. and removed dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. labels Jan 11, 2022
Co-authored-by: Maël Valais <mael@vls.dev>

Update content/en/docs/installation/upgrading/remove-deprecated-apis.md

Co-authored-by: Maël Valais <mael@vls.dev>
Signed-off-by: irbekrm <irbekrm@gmail.com>
@irbekrm irbekrm force-pushed the update_upgrade_instructions branch from a64e063 to 663378c Compare January 11, 2022 19:39
@jetstack-bot jetstack-bot added dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. and removed dco-signoff: no Indicates that at least one commit in this pull request is missing the DCO sign-off message. labels Jan 11, 2022
@maelvls
Copy link
Member

maelvls commented Jan 12, 2022

Thanks for writing this PR 🙏🙏

/lgtm

@jetstack-bot jetstack-bot added the lgtm Indicates that a PR is ready to be merged. label Jan 12, 2022
@maelvls
Copy link
Member

maelvls commented Jan 12, 2022

/unhold

@jetstack-bot jetstack-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 12, 2022
@maelvls
Copy link
Member

maelvls commented Jan 12, 2022

@irbekrm Do you think we can remove WIP from the issue title?

@irbekrm irbekrm changed the title WIP Update instructions for upgrading cert-manager resources and CRDs Update instructions for upgrading cert-manager resources and CRDs Jan 12, 2022
@jetstack-bot jetstack-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 12, 2022
@jetstack-bot jetstack-bot merged commit c43c58f into cert-manager:master Jan 12, 2022
@irbekrm irbekrm deleted the update_upgrade_instructions branch January 12, 2022 11:38
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. dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm 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.

5 participants