tilt icon indicating copy to clipboard operation
tilt copied to clipboard

live update doesn't work with jobset

Open jessicaxiejw opened this issue 1 year ago • 2 comments

Expected Behavior

Say my manifest is

apiVersion: jobset.x-k8s.io/v1alpha2
kind: JobSet
metadata:
  name: jobset-test
spec:
  replicatedJobs:
    - name: head
      template:
        spec:
          containers:
            - name: head
              image: myimage
              command: ["cat", "changes.txt"]
    - name: worker
      replicas: 2
      template:
        spec:
          containers:
            - name: worker
              image: myimage
              command: ["cat", "changes.txt"]

and the myimage's dockerfile is

# syntax=docker/dockerfile:1.4
FROM busybox:latest
COPY changes.txt /changes.txt

The tiltfile is

docker_build('myimage', '/path/to/dockerfile')
helm_resource(
  'jobset-test',
  '/path/to/jobset.yaml',
  image_deps=['myimage']
  image_keys=[('image.repository', 'image.tag')],
)

If I modify changes.txt and save, I expect head, worker-0, worker-1 pods to get live synced with the new changes.

Current Behavior

Only worker-0 and worker-1 get live updated, the head still has the older copy of changes.txt.

Steps to Reproduce

See "Expected Behavior"

Context

tilt doctor Output

$ tilt doctor
Tilt: v0.33.13, built 2024-04-26
System: darwin-arm64
---
Docker
- Host: unix:///Users/jessicaxie/.colima/docker.sock
- Server Version: 24.0.7
- API Version: 1.43
- Builder: 2
- Compose Version: v2.20.3
---
Kubernetes
- Env: unknown
- Context: <my context>
- Cluster Name: <my cluster>
- Namespace: default
- Container Runtime: cri-o
- Version: v1.27.2
- Cluster Local Registry: none
---
Thanks for seeing the Tilt Doctor!
Please send the info above when filing bug reports. 💗

The info below helps us understand how you're using Tilt so we can improve,
but is not required to ask for help.
---
Analytics Settings
--> (These results reflect your personal opt in/out status and may be overridden by an `analytics_settings` call in your Tiltfile)
- User Mode: opt-out
- Machine: 0cf4cdb1dda5938d522ff39f8c4d5974
- Repo: M2XaMpEc+34CeHQQUKDYew==

About Your Use Case

I am trying to use jobset with tilt.

jessicaxiejw avatar May 27 '24 16:05 jessicaxiejw

I think the problem is that tilt calls client.Get in tilt/internal/controllers/core/liveupdate/reconciler.go. Instead, it should call client.List when it could match multiple 🤔

The filtering is controlled by &v1alpha1.LiveUpdate{}, it returns several pods even though it only calls client.Get(). I am confused by how the filtering work though...

jessicaxiejw avatar May 27 '24 18:05 jessicaxiejw

The problem seems to come from https://github.com/tilt-dev/tilt/blob/20ab76bfa92d05b9bd34d9a727ee67cd61307e06/internal/store/k8sconv/resource.go#L174

For the head pod, pod.Owner.Name is set to jobset-test-head-0 but it is thinking that newestOwner.Name is jobset-test-worker-0.

jessicaxiejw avatar May 28 '24 19:05 jessicaxiejw

oops, sorry i missed this bug.

i think your analysis is right -- Tilt assumes that if an new replica set with the same owner starts up, then it should ignore the old replica set. But this logic shouldn't apply to JobSets that create more than one job. I think it should be enough to narrow this check to ReplicaSets

nicks avatar Oct 24 '24 03:10 nicks