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

0.13: [Bug]: Kubernetes manifest change not taken into account during deploy #4884

Closed
hnicke opened this issue Jul 24, 2023 · 5 comments
Closed

Comments

@hnicke
Copy link
Contributor

hnicke commented Jul 24, 2023

Garden Bonsai (0.13) Bug

Current Behavior

When removing a field from the kubernetes spec, like a annotation, garden does not pick up the change when deploying that action. Instead, it yields Already deployed.

Expected behavior

Garden should detect that the action changed and redeploy.
I expect the same behavior as when applying the manifest via kubectl apply.

Reproducible example

Given: A kubernetes module with one ingress, having two annotations foo and foobar:

# example.garden.yml
kind: Module
type: kubernetes
name: example
files:
  - ingress.yml
# ingress.yml
apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  name: example
  annotations:
    foo: bar
    foobar: baz
spec:
  rules:
    - host: example.com
      http:
        paths:
          - backend:
              service:
                name: example
                port:
                  number: 80
            path: /
            pathType: Prefix

Run garden deploy example.

Then, remove the foobar annotation from the ingress:

# ingress.yml
apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  name: example
  annotations:
    foo: bar
    # foobar: baz <-- remove
spec:
  rules:
    - host: example.com
      http:
        paths:
          - backend:
              service:
                name: example
                port:
                  number: 80
            path: /
            pathType: Prefix

Run garden deploy example once again.
Output: ℹ deploy.example → Already deployed.
The module is not redeployed and thus the annotation does not get removed.

Workaround

When running garden deploy example --force, the annotation is correctly removed.

Suggested solution(s)

Additional context

Your environment

  • OS: arch linux
  • How I'm running Kubernetes: GKE

garden version
0.13.9

@hnicke hnicke added the 0.13 label Jul 24, 2023
@stefreak
Copy link
Member

Thank you for reporting this! I ran into this as well when working on IRSA support (#3384) and it was really a complication.

I think the culprit here is that when removing a key from the manifest, the manifest is still a subset of the deployed manifest (See also

if (!isSubset(deployedResource, manifest)) {
)

I attempted to fix that in #3386 but some tests failed and at the time I failed to understand how things actually should work.

I think this needs a really close look @shumailxyz @vvagaytsev let me know if you need some more info

@vvagaytsev
Copy link
Collaborator

Thanks, @hnicke and @stefreak!

Yes, this is a known issue. In #4516 the logic of kubernetes status handler was updated and the call to compareDeployedResources was removed (it was retained only for container action type).

It's known that #4516 introduced some regression, so it was reverted and superseded by #4846 where we rely on the version hash comparison and particular kubernetes resources status handlers. The PR is still WIP, there is an open discussion on the better approach to solving the problem of change detection.

I think this issue can be fixed by #4846, let's try to reproduce this again once the PR is merged.

@vvagaytsev vvagaytsev removed this from Core Weekly Aug 17, 2023
@hnicke
Copy link
Contributor Author

hnicke commented Sep 12, 2023

Any updates here?
In my opinion, this bug is really, really dangerous and should get fixed.
Today I'm working on a feature toggle in our stack where when flipped, a certain host is included in a k8s ingress.
Activating the toggle works as expected - the additional host is added to the ingress.
However, when deactivating the toggle, the host remains in the ingress. 🤯
Garden will pretend that it deployed everything as declared, while it didn't.

As a user, I expect that Garden does exactly deploy what I declared, and not some alterations of it...

@shumailxyz
Copy link
Contributor

@hnicke #4846 is in final stages now and should be merged soon. There are some internal changes that we are working on before getting that PR included in the next release.

@vvagaytsev
Copy link
Collaborator

I've just re-examined this issue. It was fixed in 0.13.16 via #4846.

@vvagaytsev vvagaytsev self-assigned this Oct 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants