Implement a simple pre-uninstall check command #71

charlieegan3 · 2022-11-22T11:29:48Z

This PR is part of working on the flow defined in https://docs.google.com/document/d/1AxUss0piE9gb_r31_jBS6mv7zWALDqI0ZIfBUiJj3p0/edit?usp=sharing

Example output in a cluster that has cert-manager-csi-driver, cert-manager-csi-driver-spiffe, cert-manager-istio-csr, a Secret with Certificate owner ref, a Certificate that will be renewed and will expire soon:

irbe@jsctl$ go run main.go x clusters uninstall verify
Running checks against cluster Secrets:
        * Checking that issued certificates are safe from garbage collection...
Running checks against cluster Certificates:
        * Checking for upcoming renewals
        * Checking for upcoming expiries
        * Checking for currently failing issuances
        * Checking for unready Certificates
Running checks against cert-manager integrations installed in cluster:
        * Checking for cert-manager-istio-csr
        * Checking for cert-manager-csi-driver
        * Checking for cert-manager-csi-driver-spiffe

Results:
Secrets with Certificate owner refs were found. These Secrets will be garbage collected when Certificate CRD is uninstalled.
You can run 'jsctl experimental clusters cleanup secrets remove-certificate-owner-refs' command to remove the owner references.
        * istio-system/ca-secret secret has certificate owner ref
        * istio-system/istiod-tls secret has certificate owner ref
        * jetstack-secure/foo secret has certificate owner ref
        * jetstack-secure/spiffe-ca secret has certificate owner ref
Some certificates will be renewed soon. You might want to ensure that uninstall is completed before any renewals kick in. Or use 'cmctl renew' command to renew the certificates now.
        * istio-system/istiod certificate will be renewed soon (2023-02-07 08:41:28 +0000 GMT)
        * jetstack-secure/foo certificate will be renewed soon (2023-02-07 08:43:12 +0000 GMT)
Some certificates will expire soon. Ensure that enough time is allocated for re-installation to prevent outages. You can use 'cmctl renew' command to manually renew certs now.
        * istio-system/istiod certificate will expire soon (2023-02-07 09:11:28 +0000 GMT)
        * jetstack-secure/foo certificate will expire soon (2023-02-07 09:33:12 +0000 GMT)
A cert-manager integration that creates certificate requests was found in cluster. You might want to ensure that uninstalling Jetstack Secure software will not cause downtime.
        * cert-manager-csi-driver-spiffe found in cluster
        * cert-manager-csi-driver found in cluster
        * cert-manager-istio-csr found in cluster

irbekrm

Thanks Charlie, this looks good already!

I've not ran the command yet, but have left a bunch of comments, let me know if it makes sense.

internal/command/clusters/uninstall.go

internal/kubernetes/clients/clients.go

irbekrm · 2023-02-02T16:03:58Z

There is probably more that could be done here- if we get someone to actualy use it we can iterate and add more checks.
For now I am in favour of doing as little as possible to avoid having to maintain lots of code that is never used.

internal/kubernetes/status/components/cert_manager_csi_driver_spiffe.go

internal/command/clusters/uninstall.go

Example output: ``` $ go run main.go clusters uninstall verify The following secrets contain certificates and are owned by a Certificate resource: * test-namespace/example-net-tdst7 has certificate owner ref * test-namespace/example-com-gpsz7 has certificate owner ref * test-namespace/example-com-tls has certificate owner ref The following certificates will be renewed soon: * test-namespace/example-net will be renewed soon (13m50.20178s) The following certificates are currently being re-issued: * test-namespace/example-com The following certificate requests are pending approval or issuance: * test-namespace/example-net-dxvjx is pending approval * test-namespace/example-com-jggql is pending approval Suggested next steps: * Run 'jsctl experimental clusters cleanup secrets remove-certificate-owner-refs' to make sure secrets containing certificates are not garbage collected * Use cmctl to manually renew certificates: cmctl renew --namespace=test-namespace example-net * Wait for 1 certificates to be issued * Investigate 2 pending certificate requests ``` This command has been added to help users prepare for a migration to the operator. Signed-off-by: Charlie Egan <charlieegan3@users.noreply.github.com>

Adds a couple more checks, removes unnecessary CertificateRequests check Signed-off-by: irbekrm <irbekrm@gmail.com>

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

inteon

Thank you for the PR, I added some remarks wrt improvements that I think can be made still.

internal/command/clusters/uninstall.go

Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com>

inteon · 2023-02-08T08:50:28Z

@irbekrm I made some additional changes to fix some code style issues and a remaining bug.
Could you take a look at the latest version and also leave a review?

irbekrm

/lgtm

charlieegan3 requested a review from irbekrm November 22, 2022 11:29

jetstack-bot added the size/XXL label Nov 22, 2022

irbekrm suggested changes Nov 22, 2022

View reviewed changes

Base automatically changed from cluster-backup to main November 25, 2022 08:41

charlieegan3 force-pushed the uninstall-verify-cmd branch from 51f649b to 2451c79 Compare November 25, 2022 11:23

jetstack-bot added size/L and removed size/XXL labels Nov 25, 2022

irbekrm force-pushed the uninstall-verify-cmd branch 2 times, most recently from b4eb908 to 57bd9d4 Compare February 2, 2023 15:57

irbekrm force-pushed the uninstall-verify-cmd branch from 57bd9d4 to 525a3f9 Compare February 7, 2023 07:59

jetstack-bot added size/XXL and removed size/L labels Feb 7, 2023

irbekrm requested a review from inteon February 7, 2023 08:15

inteon force-pushed the uninstall-verify-cmd branch from 945f8ff to 6c1e5a6 Compare February 7, 2023 08:32

This was referenced Feb 7, 2023

Rename 'v1certmanager' import to 'cmapi' #83

Merged

Add x as alias for experimental #84

Merged

Add Fake client for generic Kubernetes client #85

Merged

inteon force-pushed the uninstall-verify-cmd branch from 6c1e5a6 to 7524694 Compare February 7, 2023 12:40

jetstack-bot added size/XL and removed size/XXL labels Feb 7, 2023

inteon reviewed Feb 7, 2023

View reviewed changes

internal/kubernetes/status/components/cert_manager_csi_driver_spiffe.go Outdated Show resolved Hide resolved

inteon reviewed Feb 7, 2023

View reviewed changes

internal/command/clusters/uninstall.go Outdated Show resolved Hide resolved

inteon reviewed Feb 7, 2023

View reviewed changes

internal/command/clusters/uninstall.go Show resolved Hide resolved

inteon force-pushed the uninstall-verify-cmd branch from ef87064 to e0b6267 Compare February 7, 2023 13:52

jetstack-bot added size/XXL and removed size/XL labels Feb 7, 2023

inteon force-pushed the uninstall-verify-cmd branch from e0b6267 to 5d81475 Compare February 7, 2023 13:53

jetstack-bot added size/XL and removed size/XXL labels Feb 7, 2023

charlieegan3 and others added 11 commits February 7, 2023 15:13

Update docs

5e4b043

certmanagerv1 -> cmapi

6b7676f

Improve command output

b7fb347

Update uninstall cmd docs

07162f4

Updates and refactors uninstall command

dd9f684

Adds a couple more checks, removes unnecessary CertificateRequests check Signed-off-by: irbekrm <irbekrm@gmail.com>

Adds a unit test

2046bad

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

Makes the uninstall command experimental

df71c06

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

Update docs

5447a4e

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

Reinstate output that tells user what checks are being run

3869da6

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

Fix spelling

f7eabdb

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

inteon force-pushed the uninstall-verify-cmd branch from 5d81475 to f7eabdb Compare February 7, 2023 14:13

inteon approved these changes Feb 7, 2023

View reviewed changes

internal/command/clusters/uninstall.go Outdated Show resolved Hide resolved

internal/command/clusters/uninstall.go Outdated Show resolved Hide resolved

inteon added 2 commits February 8, 2023 09:43

improve code style

6478dbd

Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com>

fix remaining bug

be09ad5

Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com>

inteon force-pushed the uninstall-verify-cmd branch from 77d594f to be09ad5 Compare February 8, 2023 08:43

irbekrm approved these changes Feb 9, 2023

View reviewed changes

jetstack-bot assigned irbekrm Feb 9, 2023

jetstack-bot added the lgtm label Feb 9, 2023

inteon merged commit bd82802 into main Feb 9, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement a simple pre-uninstall check command #71

Implement a simple pre-uninstall check command #71

charlieegan3 commented Nov 22, 2022 •

edited by irbekrm

irbekrm left a comment

irbekrm commented Feb 2, 2023 •

edited

inteon left a comment

inteon commented Feb 8, 2023

irbekrm left a comment

Implement a simple pre-uninstall check command #71

Implement a simple pre-uninstall check command #71

Conversation

charlieegan3 commented Nov 22, 2022 • edited by irbekrm

irbekrm left a comment

Choose a reason for hiding this comment

irbekrm commented Feb 2, 2023 • edited

inteon left a comment

Choose a reason for hiding this comment

inteon commented Feb 8, 2023

irbekrm left a comment

Choose a reason for hiding this comment

charlieegan3 commented Nov 22, 2022 •

edited by irbekrm

irbekrm commented Feb 2, 2023 •

edited