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

versioned-explicit-ref does not work for all resource types #984

Open
rdicroce opened this issue Jul 11, 2024 · 5 comments
Open

versioned-explicit-ref does not work for all resource types #984

rdicroce opened this issue Jul 11, 2024 · 5 comments
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-soon Must be staffed and worked on currently or soon.

Comments

@rdicroce
Copy link

What steps did you take:

  1. Deploy the below.
  2. Change test: x to test: x2.
  3. Deploy again.
---
apiVersion: batch/v1
kind: Job
metadata:
  name: foo-job
  annotations:
    kapp.k14s.io/update-strategy: always-replace
    kapp.k14s.io/change-group: job-group
    kapp.k14s.io/versioned: ""
    test: x
spec:
  template:
    spec:
      restartPolicy: Never
      containers:
        - name: busybox
          image: busybox:1.36.1
          command: 
            - /bin/sh
            - -ec
            - sleep 5
---
apiVersion: apps/v1
kind: Deployment
metadata:
  name: simple-app
  annotations:
    kapp.k14s.io/change-rule: upsert after upserting job-group
    kapp.k14s.io/versioned-explicit-ref: |
      apiVersion: batch/v1
      kind: Job
      name: foo-job
spec:
  selector:
    matchLabels:
      simple-app: ""
  template:
    metadata:
      labels:
        simple-app: ""
    spec:
      containers:
      - name: simple-app
        image: busybox:1.36.1
        command:
          - /bin/sh
          - -ec
          - while :; do echo '.'; sleep 5 ; done

What happened:
The Job was updated, but the Deployment was not.

What did you expect:
The Deployment should have been updated after the Job, because of the change-rule and the versioned-explicit-ref.

Anything else you would like to add:
From looking at the kapp docs, it seems like the Deployment's versioned-explicit-ref annotation is supposed to contain the versioned name when it's written to Kubernetes. But it doesn't - the Kubernetes dashboard shows the annotation still has the unversioned name.

Environment:

  • kapp version (use kapp --version): 0.63.1
  • OS (e.g. from /etc/os-release): Windows 10
  • Kubernetes version (use kubectl version): 1.30.0

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.

@rdicroce rdicroce added bug This issue describes a defect or unexpected behavior carvel triage This issue has not yet been reviewed for validity labels Jul 11, 2024
@100mik
Copy link
Contributor

100mik commented Jul 12, 2024

I think what happening here is a weird interaction between the annotation kapp.k14s.io/update-strategy: always-replace and the explicit-version-ref. Mainly cause versioning happens on update and according to the strategy the job is always created again.
I will reproduce this locally and take a peek at what happens.

What is your reason for using the update strategy?

@100mik 100mik added helping with an issue Debugging happening to identify the problem carvel accepted This issue should be considered for future work and that the triage process has been completed bug This issue describes a defect or unexpected behavior and removed carvel triage This issue has not yet been reviewed for validity bug This issue describes a defect or unexpected behavior labels Jul 12, 2024
@rdicroce
Copy link
Author

I'm new to K8s and kapp, and I discovered quickly that K8s won't allow some fields to be modified by an update. Hence, the Job has to be deleted and recreated.

@100mik
Copy link
Contributor

100mik commented Jul 24, 2024

I discovered quickly that K8s won't allow some fields to be modified by an update.
Yep, this would be true for spec.Template and I believe you will be trying to bump the images here.

This would be a valid use case, I am curious about if it is indeed the cause.
To have a path forward today, we could rely on a fixed tag, and use the default update strategy, however, I am not saying that we should not improve this behaviour.

@rdicroce
Copy link
Author

Right now, I'm working around the problem by:

  1. Putting an annotation on both objects.
  2. Creating a "constants" file that contains a value for the annotation.
  3. Referencing that constant as the value for both annotations.

Then, if either object needs to be updated, I change the constant. It's basically a manual way of doing what explicit-version-ref is supposed to do. It works, but it's clunky.

@rdicroce
Copy link
Author

After some more experimentation, I've determined that the update-strategy does not matter. If you remove it in the original example I posted above, the explicit-version-ref still doesn't work. And if you replace the Job with a ConfigMap that uses the always-replace strategy (see below), it works fine. And if you replace the Job with a Service, it half works: the Service has -ver-x appended to its name, but the explicit-version-ref annotation on the Deployment does not.

---
apiVersion: v1
kind: ConfigMap
metadata:
  name: config-1
  annotations:
    kapp.k14s.io/versioned: ""
    kapp.k14s.io/update-strategy: always-replace
    kapp.k14s.io/change-group: job-group
    test: x
data:
  foo: bar
---
apiVersion: apps/v1
kind: Deployment
metadata:
  name: simple-app
  annotations:
    kapp.k14s.io/change-rule: upsert after upserting job-group
    kapp.k14s.io/versioned-explicit-ref: |
      apiVersion: v1
      kind: ConfigMap
      name: config-1
spec:
  selector:
    matchLabels:
      simple-app: ""
  template:
    metadata:
      labels:
        simple-app: ""
    spec:
      containers:
      - name: simple-app
        image: busybox:1.36.1
        command:
          - /bin/sh
          - -ec
          - while :; do echo '.'; sleep 5 ; done

@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 Jul 24, 2024
@100mik 100mik added bug This issue describes a defect or unexpected behavior priority/important-soon Must be staffed and worked on currently or soon. and removed bug This issue describes a defect or unexpected behavior helping with an issue Debugging happening to identify the problem priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. labels Aug 1, 2024
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-soon Must be staffed and worked on currently or soon.
Projects
Status: To Triage
Development

No branches or pull requests

3 participants