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

Kapp shows "update" operation for ConfigMap that has no changes #603

Open
ChristianCiach opened this issue Sep 12, 2022 · 10 comments
Open
Labels
bug This issue describes a defect or unexpected behavior carvel accepted This issue should be considered for future work and that the triage process has been completed priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete.

Comments

@ChristianCiach
Copy link

What steps did you take:

I have a simple, static ConfigMap where kapp insists that deploying the ConfigMap will result in an update operation, although the ConfigMap is exactly the same as the deployed one. If I deploy the file multiple times in a row, kapp still announces the update operation every time.

What did you expect:

When the ConfigMap has not changed, kapp should not show an update operation.

Anything else you would like to add:

Using kapp deploy -c shows the removal of an empty metadata.annotations element:

kapp deploy -a clustering  -f configmap.yaml -c
Target cluster 'https://127.0.0.1:6443' (nodes: mngr1.censored 8+)

@@ update configmap/clustering-cacerts-m7b68mk2mm (v1) namespace: default @@
  ...
4776,4776   metadata:
4777     -   annotations: {}
4778,4777     creationTimestamp: "2022-09-12T14:26:59Z"
4779,4778     labels:

Changes

Namespace  Name                           Kind       Age  Op      Op st.  Wait to    Rs  Ri  
default    clustering-cacerts-m7b68mk2mm  ConfigMap  16m  update  -       reconcile  ok  -  

I don't know where this annotations field comes from. The metadata of our ConfigMap does not include an annotations field:

kind: ConfigMap
metadata:
  labels:
    app.kubernetes.io/part-of: clustering
  name: clustering-cacerts-m7b68mk2mm
  namespace: default

Kapp seems to think that the absence of an annotations field is different from an empty one.

Environment:

  • kapp version (use kapp --version): kapp version 0.52.0
  • OS (e.g. from /etc/os-release): AlmaLinux 8.6 (Sky Tiger)
  • Kubernetes version (use kubectl version): Server Version: version.Info{Major:"1", Minor:"24", GitVersion:"v1.24.1+k3s1", GitCommit:"0581808f5c160b0c0cafec5b8f20430835f34f44", GitTreeState:"clean", BuildDate:"2022-06-11T17:26:28Z", GoVersion:"go1.18.1", Compiler:"gc", Platform:"linux/amd64"}

Vote on this request

This is an invitation to the community to vote on issues, to help us prioritize our backlog. Use the "smiley face" up to the right of this comment to vote.

👍 "I would like to see this addressed as soon as possible"
👎 "There are other more important things to focus on right now"

We are also happy to receive and review Pull Requests if you want to help working on this issue.

@ChristianCiach ChristianCiach added bug This issue describes a defect or unexpected behavior carvel triage This issue has not yet been reviewed for validity labels Sep 12, 2022
@ChristianCiach ChristianCiach changed the title Kapp shows "update" oepration for ConfigMap that has no changes Kapp shows "update" operation for ConfigMap that has no changes Sep 12, 2022
@ChristianCiach
Copy link
Author

The actual deployment of the not-really-changed ConfigMap is very fast, less than a second:

3:00:12PM: ---- applying 1 changes [0/1 done] ----
3:00:12PM: update configmap/clustering-cacerts-m7b68mk2mm (v1) namespace: default
3:00:12PM: ---- waiting on 1 changes [0/1 done] ----
3:00:12PM: ok: reconcile configmap/clustering-cacerts-m7b68mk2mm (v1) namespace: default
3:00:12PM: ---- applying complete [1/1 done] ----
3:00:12PM: ---- waiting complete [1/1 done] ----

@ChristianCiach
Copy link
Author

Kapp version 0.51.0 behaves the same, so this is not a very new bug.

@praveenrewar
Copy link
Member

Hi @ChristianCiach! I am not able to reproduce the issue with kapp version 0.52.0 and kubernetes version 1.24. Do you have any kapp config (like rebase rules) along with the manifest or are there any external factors that could effect this resource?

I don't know where this annotations field comes from. The metadata of our ConfigMap does not include an annotations field

kapp adds annotations to store the original resource (kapp.k14s.io/original) and the diff md5 (kapp.k14s.io/original-diff-md5).

@ChristianCiach
Copy link
Author

ChristianCiach commented Sep 12, 2022

Thank you for your fast response!

Well, we do have a custom kapp config, but I can reproduce this issue even without those. This is very confusing. I will try to investigate this further tomorrow and I will report back then.

@ChristianCiach
Copy link
Author

There is something inside the ConfigMap contents that triggers this issue. It is a root-certiticate bundle and after stripping more and more certificates from the ConfigMap, the issue eventually disappears. I don't think the file is confidential, so I will upload it after a quick verification.

@ChristianCiach
Copy link
Author

ChristianCiach commented Sep 12, 2022

Here you go: https://gist.github.com/ChristianCiach/3ac9b7261ab9b14d7cdefe7b171b542f

Can you reproduce the issue when deploying this file using kapp deploy -a app-name -f configmap-with-bug.yaml?

By the way, the file has been automatically created from a cacert.pem file using a Kustomize configMapGenerator.

@praveenrewar
Copy link
Member

Yeah, I see what's happening. We add the kapp.k14s.io/original annotation for every resource, but if the kapp.k14s.io/disable-original annotation is present or if the resource size is large then this annotation is not added (as it would exceed the maximum size allowed for an annotation). Right now it seems to be the latter case and I think this is a bug because of which this diff is being produced every time. It doesn't happen when I manually add the kapp.k14s.io/disable-original annotation (you could maybe use this as a workaround until this is resolved).

@praveenrewar praveenrewar added carvel accepted This issue should be considered for future work and that the triage process has been completed and removed carvel triage This issue has not yet been reviewed for validity labels Sep 12, 2022
@ChristianCiach
Copy link
Author

According to #410 (comment) , adding kapp.k14s.io/disable-original may also trigger the issue I am currently seeing:

Redeploying with the unchanged configuration file will show operation as an update due to dumb diffing.

This issue seems to be purely cosmetic, so I am okay with not doing anything on my side for the time being,.

@praveenrewar
Copy link
Member

It's true that adding the kapp.k14s.io/disable-original could result in unwanted diffs because the kapp.k14s.io/original annotation is used to calculate diff in a much smarter way. Although, right now, the reason is a bit different.
kapp adds the kapp.k14s.io/original, kapp.k14s.io/original-diff-md5 and the kapp.k14s.io/identity annotation for all the resources. The identity annotation is removed as part of diffing and since the other 2 annotations are already absent, it causes the removal of the empty annotations: {}, so even if we add any random annotation and not just the disable-original annotation, I think the diff would be gone as there will be some annotation present in the manifest. We can also use a rebase rule to copy the existing annotations.

@ChristianCiach
Copy link
Author

ChristianCiach commented Sep 12, 2022

Thanks so far! I can confirm that adding the annotation removes the bogus update operation in this case. Of course it would be nice to see this fixed properly some day, but this works fine for us for the time being. Thanks so much for your quick help!

@renuy renuy added the priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. label Sep 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue describes a defect or unexpected behavior carvel accepted This issue should be considered for future work and that the triage process has been completed priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete.
Projects
Status: To Triage
Development

No branches or pull requests

3 participants