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

Update Kustomize templates to 5.0.x #955

Open
burmanm opened this issue May 3, 2023 · 8 comments
Open

Update Kustomize templates to 5.0.x #955

burmanm opened this issue May 3, 2023 · 8 comments
Assignees
Labels
blocked Issues in the state 'blocked' enhancement New feature or request refactoring

Comments

@burmanm
Copy link
Contributor

burmanm commented May 3, 2023

What is missing?
We use deprecated Kustomize template directives. We should update to 5.0.x compatible ones to prevent any deprecation warnings when deploying.

Why do we need it?

Environment

  • K8ssandra Operator version:

    Insert image tag or Git SHA here

Anything else we need to know?:

@burmanm burmanm added enhancement New feature or request refactoring labels May 3, 2023
@burmanm burmanm mentioned this issue May 3, 2023
5 tasks
@adejanovski adejanovski added ready-for-review Issues in the state 'ready-for-review' in-progress Issues in the state 'in-progress' and removed ready-for-review Issues in the state 'ready-for-review' labels May 3, 2023
@Miles-Garnsey
Copy link
Member

The problem

I am still researching the best way to solve this given that vars are being deprecated and the kustomize project have not provided adequate solutions (e.g. discussions here and here) to allow replacing fields with namespaces from the top level kustomization of a multi-namespace kustomize deployment.

The problem we are facing appears to reside here, where we define a set of vars CERTIFICATE_NAME/NAMESPACE and SERVICE_NAME/NAMESPACE. These are then used to inject a cert into a ValidatingWebhookConfiguration with cert details here and a certificate with service details here. (The CRD is also involved, but let's focus on the above two for now).

When converting to replacements from vars here @burmanm attempted to manipulate the ValidatingWebhookConfiguration and CustomResourceDefinition to take the namespace/name of a Certificate, and add the namespace and name of a service to the DNS names of a Certificate. This replicates the vars functionality discussed above. (There is some more manipulation on CRDs as well, but leave that for now.)

At present, having diff'd the output of the two kustomizations, I see a problem here (correct version on the left):

apiVersion: admissionregistration.k8s.io/v1                             apiVersion: admissionregistration.k8s.io/v1
kind: ValidatingWebhookConfiguration                                    kind: ValidatingWebhookConfiguration
metadata:                                                               metadata:
  annotations:                                                            annotations:
    cert-manager.io/inject-ca-from: cass-operator/cass-op       |           cert-manager.io/inject-ca-from: system/serving-cert
  creationTimestamp: null                                       <
  name: cass-operator-validating-webhook-configuration                    name: cass-operator-validating-webhook-configuration
webhooks:                                                               webhooks:
- admissionReviewVersions:                                              - admissionReviewVersions:
  - v1                                                                    - v1
  - v1beta1                                                               - v1beta1
  clientConfig:                                                           clientConfig:

And another here:

apiVersion: cert-manager.io/v1                                          apiVersion: cert-manager.io/v1
kind: Certificate                                                       kind: Certificate
metadata:                                                               metadata:
  name: cass-operator-serving-cert                                        name: cass-operator-serving-cert
  namespace: cass-operator                                                namespace: cass-operator
spec:                                                                   spec:
  dnsNames:                                                               dnsNames:
  - cass-operator-webhook-service.cass-operator.svc             |         - webhook-service.system.svc
  - cass-operator-webhook-service.cass-operator.svc.clust       |         - webhook-service.system.svc.cluster.local
  issuerRef:                                                              issuerRef:
    kind: Issuer                                                            kind: Issuer
    name: cass-operator-selfsigned-issuer                                   name: cass-operator-selfsigned-issuer

Michael's analysis is that the replacements are occuring prior to the namespace transformer running. This is odd because the namespace transformer is in an overlay while the replacements reside in a component. As mentioned in this comment, the KEP for components states that:

A kustomization that is marked as a Component has basically the same capabilities as a normal kustomization. The main distinction is that they are evaluated after the resources of the parent kustomization (overlay or component) have been accumulated, and on top of them. This means that:

And currently replacements within components do not seem to be adhering to that contract, as they are being run prior to the namespace transformer in the bases.

Research so far

I have a few preliminary suggestions/observations/directions for further research;

  1. How concerned about this really are we given (a) the project's history of making overly-restrictive decisions and then unwinding them in response to community concerns, and (b) the project's release cadence? (On my understanding, final removal of vars wouldn't happen until kustomize 6, where 5 has just been released and the previous cycle was 2 years between major releases. The kustomize fix command for removing vars is still marked experimental.) Let's keep some perspective, whatever the outcome.
  2. A lot of our issues concern webhooks and certs. Is it best practice for us to be creating webhooks in the operator's namespace? Especially for k8s native resources (e.g. pods, sts) it seems that it may be best practice to hard code these to the kube-system namespace. This avoids issues where native resource creation will be blocked due to operator uninstallation (when the webhook resource is not deleted concurrently).
  3. Can we use the new setters functionality for namespace control from the top level?
  4. If having this in a component doesn't work, maybe we should experiment with putting it in an overlay instead. There is a distinction in order-of-operations depending on whether a particular transform exists in an overlay or a component - see here, so maybe this is just switched in the current version.
  5. Realistically, I think I will re-post the above analysis on the kustomize project's issues board, because this behaviour seems straight-forwardly wrong given the KEP for components.

@Miles-Garnsey
Copy link
Member

@burmanm I'm still trying to work through what the problem is here. I was drafting an issue for the kustomize project, but when I put together a minimal reproducible example (see here) I found that the following set up works;

  1. Create a base with a service and a cert referenced via resources.
  2. Create an overlay referencing the base, with a namespace transformer in it.
  3. Create a component with the replacements in it.

Can you please test against that repo and see whether you get the expected result? The dnsNames field should but updated to the namespace of the service AFTER it has been processed by the namespace transformer (new-namespace it the expected result, and that's what we get).

@Miles-Garnsey
Copy link
Member

Michael and I discussed again, it looks like (while setting the namespace in an overlay works), setting the namespace in the top level kustomization (when it also includes the component) does not.

For re-use in k8ssandra-operator, this will require us to copy across the namespacing replacements from cass-operator to k8ssandra-operator.

It also suggests that kustomize has broken the components API for replacements, since components were meant to run after their parent overlays.

I have raised an issue on the repo here, but will not work further on this at present, because it seems like a bug in kustomize. I also tried:

  1. Using setters.
  2. Trying to patch the namespace (instead of using a namespace transformer) from within an overlay, which should run prior to the component being run (and the component should therefore pick up the namespace change. This didn't work either. Replacements are the problem.

There are additional things I could try, but I think two days is enough to spend on this. My reasoning for putting it on hold is that;

  1. Var deprecation is still in an almost experimental state. E.g. the kustomize fix migration is still experimental. So while the project may be keen for people to migrate to replacements, they are not actually ready to support this yet.
  2. Vars will never be removed from the v1beta1 API of kustomize.
  3. Even if the v1 API came out in kustomize 6.0 (which would not be released until Feb 2025 based on the historical release cadence), the v1beta1 API would not be removed for two versions (which would be 2027, again based on historical cadence).

@adejanovski
Copy link
Contributor

@burmanm @Miles-Garnsey, if I understand correctly we're blocked here until Kustomize fixes some behaviors (if that ever happens)?
What's our path forward with this? Move the ticket to blocked for now and revisit this in the future?

@Miles-Garnsey
Copy link
Member

There's a response on the issue I raised, I need to test their latest fix.

@adejanovski adejanovski added blocked Issues in the state 'blocked' and removed in-progress Issues in the state 'in-progress' labels Oct 10, 2023
@burmanm
Copy link
Contributor Author

burmanm commented Apr 11, 2024

@Miles-Garnsey This is the original ticket.

@burmanm
Copy link
Contributor Author

burmanm commented Apr 11, 2024

@adejanovski So, here's a short description of the problem. We currently ship with only kustomize 4.x support in k8ssandra-operator, however if users want to use kubectl to install k8ssandra-operator they can only use kustomize 5.x and these are not compatible with each other in cases such as namespaces or component execution order.

Last kubectl that supported kustomize 4.x was 1.26, which was EOLed in February 2024. Most users probably use 1.28 or 1.29 or something newer as that's what their package manager (such as brew) installs.

If we stick to kustomize 4.x series, we'll be in a situation where we should mark use of kustomize as internal only and remove it from our docs as a method of installation. That means helm would be the only supported method of installation.

There's also the 5.0.x and 5.4.x not being entirely compatible with each other, making kustomize an even bigger mess of course.

@adejanovski
Copy link
Contributor

Is Kustomize now in a state that allows us to upgrade without breaking our installations? I'd jump straight to the latest (5.4?), which could mean kubectl -k would not work for a bit, but we can live with it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Issues in the state 'blocked' enhancement New feature or request refactoring
Projects
Status: Blocked/Stale
Development

No branches or pull requests

3 participants