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

[BOP-14][BOP-40] Support for deleting addon helm charts, namespaced addon helm charts #3

Merged
merged 3 commits into from
Nov 3, 2023

Conversation

tppolkow
Copy link
Contributor

@tppolkow tppolkow commented Nov 1, 2023

This PR makes it so the namespace specified in the blueprint for Addons is the one that is used for deploying the helm chart. The Addon CRD itself is deployed in the boundless-system namespace.

Additionally this PR adds support for deletion of addons. When an addon is removed from a blueprint and we run update with the modified blueprint, the addon that is no longer present in the blueprint should now be deleted.

Testing

Manually tested

Step 1. Apply blueprint with grafana addon

    addons:
      - name: my-grafana
        enabled: true
        kind: HelmAddon
        namespace: monitoring
        chart:
          name: grafana
          repo: https://grafana.github.io/helm-charts
          version: 6.58.7
          values: |
            ingress:
              enabled: true

Result:

tpolkowski@tpolkowski-MBP16-1947 boundless-cli % k get addon -A
NAMESPACE          NAME         AGE
boundless-system   my-grafana   2s
tpolkowski@tpolkowski-MBP16-1947 boundless-cli % k get helmchart -A
NAMESPACE          NAME            JOB                          CHART           TARGETNAMESPACE    VERSION   REPO                                         HELMVERSION   BOOTSTRAP
boundless-system   ingress-nginx   helm-install-ingress-nginx   ingress-nginx   boundless-system   4.7.1     https://kubernetes.github.io/ingress-nginx
monitoring         grafana         helm-install-grafana         grafana         monitoring         6.58.7    https://grafana.github.io/helm-charts
tpolkowski@tpolkowski-MBP16-1947 boundless-cli % k get all -n default
NAME                 TYPE        CLUSTER-IP   EXTERNAL-IP   PORT(S)   AGE
service/kubernetes   ClusterIP   10.96.0.1    <none>        443/TCP   3m45s
tpolkowski@tpolkowski-MBP16-1947 boundless-cli % k get all -n monitoring
NAME                             READY   STATUS    RESTARTS   AGE
pod/helm-install-grafana-fm992   1/1     Running   0          13s

NAME                             COMPLETIONS   DURATION   AGE
job.batch/helm-install-grafana   0/1           13s        13s

The Addon CRD is deployed in boundless-system namespace, whereas the grafana helmchart is correctly deployed in monitoring namespace.

Step 2: Update the namespace to monitoring2 and apply again.

Result:

tpolkowski@tpolkowski-MBP16-1947 boundless-cli % k get addon -A
NAMESPACE          NAME         AGE
boundless-system   my-grafana   65s
tpolkowski@tpolkowski-MBP16-1947 boundless-cli % k get helmchart -A
NAMESPACE          NAME            JOB                          CHART           TARGETNAMESPACE    VERSION   REPO                                         HELMVERSION   BOOTSTRAP
boundless-system   ingress-nginx   helm-install-ingress-nginx   ingress-nginx   boundless-system   4.7.1     https://kubernetes.github.io/ingress-nginx
monitoring2        grafana         helm-install-grafana         grafana         monitoring2        6.58.7    https://grafana.github.io/helm-charts
tpolkowski@tpolkowski-MBP16-1947 boundless-cli % k get all -n monitoring
No resources found in monitoring namespace.
tpolkowski@tpolkowski-MBP16-1947 boundless-cli % k get all -n monitoring2
NAME                             READY   STATUS      RESTARTS   AGE
pod/grafana-66d4dc9f56-fb2d4     1/1     Running     0          19s
pod/helm-install-grafana-sbxh2   0/1     Completed   3          72s

NAME              TYPE        CLUSTER-IP      EXTERNAL-IP   PORT(S)   AGE
service/grafana   ClusterIP   10.96.242.218   <none>        80/TCP    19s

NAME                      READY   UP-TO-DATE   AVAILABLE   AGE
deployment.apps/grafana   1/1     1            1           19s

NAME                                 DESIRED   CURRENT   READY   AGE
replicaset.apps/grafana-66d4dc9f56   1         1         1       19s

NAME                             COMPLETIONS   DURATION   AGE
job.batch/helm-install-grafana   1/1           57s        72s

The helm chart has now been moved to monitoring2 namespace.

Step 3: Now remove the addons section and update again.

Result:

tpolkowski@tpolkowski-MBP16-1947 boundless-cli % bctl update --config blueprint-kind.yaml
INFO[0000] Updating Components
INFO[0000] Applying Blueprint
tpolkowski@tpolkowski-MBP16-1947 boundless-cli % k get addon -A
No resources found
tpolkowski@tpolkowski-MBP16-1947 boundless-cli % k get helmchart -A
NAMESPACE          NAME            JOB                          CHART           TARGETNAMESPACE    VERSION   REPO                                         HELMVERSION   BOOTSTRAP
boundless-system   ingress-nginx   helm-install-ingress-nginx   ingress-nginx   boundless-system   4.7.1     https://kubernetes.github.io/ingress-nginx
monitoring2        grafana         helm-delete-grafana          grafana         monitoring2        6.58.7    https://grafana.github.io/helm-charts
tpolkowski@tpolkowski-MBP16-1947 boundless-cli % k get all -n monitoring2
No resources found in monitoring2 namespace.
tpolkowski@tpolkowski-MBP16-1947 boundless-cli % k get all -n monitoring
No resources found in monitoring namespace.

Addons are cleaned up, nothing exists in either monitoring or monitoring2 namespace. The namespaces do still exist. I'm not sure at this point how we want to handle clean up of namespaces if we have no addons in those namespaces. Since it is possible users created something of their own in that namespace in the meantime.

Testing multiple of the same helm charts

      - name: my-grafana
        enabled: true
        kind: HelmAddon
        namespace: monitoring
        chart:
          name: grafana
          repo: https://grafana.github.io/helm-charts
          version: 6.58.7
          values: |
            ingress:
              enabled: true
      - name: example1
        kind: HelmAddon
        enabled: true
        namespace: default
        chart:
          name: nginx
          repo: https://charts.bitnami.com/bitnami
          version: 15.1.1
          values: |
            "service":
              "type": "ClusterIP"
      - name: example2
        kind: HelmAddon
        enabled: true
        namespace: mynamespace
        chart:
          name: nginx
          repo: https://charts.bitnami.com/bitnami
          version: 15.1.1
          values: |
            "service":
              "type": "ClusterIP"

Result:

tpolkowski@tpolkowski-MBP16-1947 boundless-cli % k get addon -A
NAMESPACE          NAME         AGE
boundless-system   example1     25s
boundless-system   example2     25s
boundless-system   my-grafana   25s
tpolkowski@tpolkowski-MBP16-1947 boundless-cli % k get helmchart -A
NAMESPACE          NAME            JOB                          CHART           TARGETNAMESPACE    VERSION   REPO                                         HELMVERSION   BOOTSTRAP
boundless-system   ingress-nginx   helm-install-ingress-nginx   ingress-nginx   boundless-system   4.7.1     https://kubernetes.github.io/ingress-nginx
default            nginx           helm-install-nginx           nginx           default            15.1.1    https://charts.bitnami.com/bitnami
monitoring         grafana         helm-install-grafana         grafana         monitoring         6.58.7    https://grafana.github.io/helm-charts
mynamespace        nginx           helm-install-nginx           nginx           mynamespace        15.1.1    https://charts.bitnami.com/bitnami
tpolkowski@tpolkowski-MBP16-1947 boundless-cli % k get all -n mynamespace
NAME                           READY   STATUS      RESTARTS   AGE
pod/helm-install-nginx-xzc6g   0/1     Completed   0          71s
pod/nginx-769c898b4f-njp2n     1/1     Running     0          46s

NAME            TYPE        CLUSTER-IP     EXTERNAL-IP   PORT(S)   AGE
service/nginx   ClusterIP   10.96.32.236   <none>        80/TCP    46s

NAME                    READY   UP-TO-DATE   AVAILABLE   AGE
deployment.apps/nginx   1/1     1            1           46s

NAME                               DESIRED   CURRENT   READY   AGE
replicaset.apps/nginx-769c898b4f   1         1         1       46s

NAME                           COMPLETIONS   DURATION   AGE
job.batch/helm-install-nginx   1/1           28s        71s
tpolkowski@tpolkowski-MBP16-1947 boundless-cli % k get all -n default
NAME                           READY   STATUS      RESTARTS   AGE
pod/helm-install-nginx-wcsf6   0/1     Completed   0          74s
pod/nginx-769c898b4f-x94r9     1/1     Running     0          48s

NAME                 TYPE        CLUSTER-IP     EXTERNAL-IP   PORT(S)   AGE
service/kubernetes   ClusterIP   10.96.0.1      <none>        443/TCP   2m7s
service/nginx        ClusterIP   10.96.111.89   <none>        80/TCP    48s

NAME                    READY   UP-TO-DATE   AVAILABLE   AGE
deployment.apps/nginx   1/1     1            1           48s

NAME                               DESIRED   CURRENT   READY   AGE
replicaset.apps/nginx-769c898b4f   1         1         1       48s

NAME                           COMPLETIONS   DURATION   AGE
job.batch/helm-install-nginx   1/1           29s        74s

And deleting example addon deletes just the addon we want:

tpolkowski@tpolkowski-MBP16-1947 boundless-cli % k get addon -A
NAMESPACE          NAME         AGE
boundless-system   example2     118s
boundless-system   my-grafana   118s
tpolkowski@tpolkowski-MBP16-1947 boundless-cli % k get helmchart -A
NAMESPACE          NAME            JOB                          CHART           TARGETNAMESPACE    VERSION   REPO                                         HELMVERSION   BOOTSTRAP
boundless-system   ingress-nginx   helm-install-ingress-nginx   ingress-nginx   boundless-system   4.7.1     https://kubernetes.github.io/ingress-nginx
monitoring         grafana         helm-install-grafana         grafana         monitoring         6.58.7    https://grafana.github.io/helm-charts
mynamespace        nginx           helm-install-nginx           nginx           mynamespace        15.1.1    https://charts.bitnami.com/bitnami
tpolkowski@tpolkowski-MBP16-1947 boundless-cli % k get all -n default
NAME                 TYPE        CLUSTER-IP   EXTERNAL-IP   PORT(S)   AGE
service/kubernetes   ClusterIP   10.96.0.1    <none>        443/TCP   2m57s

@tppolkow tppolkow force-pushed the BOP-14 branch 2 times, most recently from 7e6af6c to 25a0344 Compare November 2, 2023 20:58
@tppolkow tppolkow changed the title Support for deleting addon helm charts, namespaced addon helm charts … [BOP-14][BOP-40] Support for deleting addon helm charts, namespaced addon helm charts Nov 2, 2023
@tppolkow tppolkow force-pushed the BOP-14 branch 3 times, most recently from be95b05 to 1427ded Compare November 2, 2023 22:06
…BOP-14,BOP-46

Cleanup code and use boundless-system namespace for addon CRDs BOP-14/BOP-40

Fixup deletion to work with namespaces
@tppolkow tppolkow marked this pull request as ready for review November 3, 2023 15:29
Copy link
Collaborator

@ranyodh ranyodh left a comment

Choose a reason for hiding this comment

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

Awesome first PR! LGTM

controllers/blueprint_controller.go Show resolved Hide resolved
Copy link
Collaborator

@nwneisen nwneisen left a comment

Choose a reason for hiding this comment

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

Looks good overall! Just a few small thoughts. Nothing to stop you from merging on. I'm going to take it for a spin also

controllers/blueprint_controller.go Show resolved Hide resolved
controllers/blueprint_controller.go Outdated Show resolved Hide resolved
pkg/helm/chart.go Show resolved Hide resolved
pkg/helm/chart.go Outdated Show resolved Hide resolved
@tppolkow tppolkow merged commit 98f5efc into main Nov 3, 2023
3 checks passed
@tppolkow tppolkow deleted the BOP-14 branch November 3, 2023 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants