[k8s] Update waiting logic for init containers #3762

romilbhardwaj · 2024-07-18T18:58:27Z

Closes #3702.

Tested with:

Base case - bad image, no init container, returns clear error message: sky launch -c test --cloud kubernetes --image-id romilb/fakeimage -- echo hi

run_instances: Error occurred when creating pods: Failed to create container while launching the node. Error details: failed to pull and unpack image "docker.io/romilb/fakeimage:latest": failed to resolve reference "docker.io/romilb/fakeimage:latest": pull access denied, repository does not exist or may require authorization: server message: insufficient_scope: authorization failed.

Healthy init container - sky launch -c test --cloud kubernetes with this config.yaml:

kubernetes:
  pod_config:
    spec:
      initContainers:
        - name: init-myservice
          image: busybox
          command: ['sh', '-c', 'echo hi']

Bad init container - sky launch -c test --cloud kubernetes with this config.yaml:

kubernetes:
  pod_config:
    spec:
      initContainers:
        - name: init-myservice
          image: busybox
          command: ['sh', '-c', 'exit 1']

W 07-18 11:55:11 instance.py:604] run_instances: Error occurred when creating pods: Failed to run init container for pod test-2ea4-head. Error details: {'container_id': 'containerd://88a2f346c4b2b5abc09493cb7204d1bcd91eca0fa35670d9142b5c6840e00610',
W 07-18 11:55:11 instance.py:604]  'exit_code': 1,
W 07-18 11:55:11 instance.py:604]  'finished_at': datetime.datetime(2024, 7, 18, 18, 55, 11, tzinfo=tzutc()),
W 07-18 11:55:11 instance.py:604]  'message': None,
W 07-18 11:55:11 instance.py:604]  'reason': 'Error',
W 07-18 11:55:11 instance.py:604]  'signal': None,
W 07-18 11:55:11 instance.py:604]  'started_at': datetime.datetime(2024, 7, 18, 18, 55, 11, tzinfo=tzutc())}.

…o init_container_fix

Michaelvll

Thanks @romilbhardwaj! Left several coments.

sky/provision/kubernetes/instance.py

Michaelvll · 2024-07-24T21:36:38Z

sky/provision/kubernetes/instance.py

+                raise config_lib.KubernetesError(
+                    f'Failed to run init container for pod {pod.metadata.name}.'
+                    f' Error details: {msg}.')
+            init_waiting = init_status.state.waiting


nit: do we still need to check init_waiting if init_terminated is true?

Good point, checking init_waiting is not required when init_terminated is not None. Updated.

Oh, should this be return or continue? Will there be multiple init_containers?

Oops good catch - should be continue

Michaelvll · 2024-07-24T21:39:02Z

sky/provision/kubernetes/instance.py

+            if (init_waiting is not None and init_waiting.reason
+                    not in ['ContainerCreating', 'PodInitializing']):


Curious, if init_waiting.reason is not in these two states, what else states it could be in, and why do we directly fail for those cases?

It would be great if we can have a reference link to those states in the comment.

Unfortunately there's no listing of state.reason available (it's marked as a str without a list of enum values). Asked an AI assistant and it listed these states:

ContainerCreating: The container is still in the process of being created. CrashLoopBackOff: The container is repeatedly crashing and Kubernetes is backing off before restarting it again. ErrImagePull: There was an error pulling the container image from the container registry. ImagePullBackOff: Kubernetes is backing off from pulling the image after a series of failures. CreateContainerConfigError: There was an error in creating the container configuration. InvalidImageName: The provided image name is invalid. PodInitializing: The pod is in the process of initializing. RunContainerError: There was an error running the container. ContainerCannotRun: The container cannot run, possibly due to a command or configuration error. ErrImageNeverPull: The image was not pulled because the policy is set to never pull. NetworkPluginNotReady: The network plugin is not ready for the container. BackOff: Kubernetes is backing off from restarting the container.

I don't fully trust this list so I don't want to include it in comments, but given this looks like the only successful states are ContainerCreating and PodInitializing...

Should we add a TODO here saying that there might be other states we need to include here when it occurs during usage?

Good idea - added now.

Michaelvll

Thanks for the update @romilbhardwaj! LGTM except for the two comments below : )

Michaelvll · 2024-07-24T22:28:17Z

sky/provision/kubernetes/instance.py

+                raise config_lib.KubernetesError(
+                    f'Failed to run init container for pod {pod.metadata.name}.'
+                    f' Error details: {msg}.')
+            init_waiting = init_status.state.waiting


Oh, should this be return or continue? Will there be multiple init_containers?

Michaelvll · 2024-07-24T22:29:02Z

sky/provision/kubernetes/instance.py

+            if (init_waiting is not None and init_waiting.reason
+                    not in ['ContainerCreating', 'PodInitializing']):


Should we add a TODO here saying that there might be other states we need to include here when it occurs during usage?

romilbhardwaj · 2024-07-24T22:43:53Z

Thanks @Michaelvll - will re-run the manual tests then merge.

romilbhardwaj added 2 commits July 18, 2024 09:13

minor fix

7048b0d

init container fix

d1b275e

romilbhardwaj mentioned this pull request Jul 18, 2024

[k8s] unable to launch pod with init container #3702

Closed

romilbhardwaj added 2 commits July 18, 2024 12:02

init container fix

727a3cb

Merge branch 'master' of https://github.com/skypilot-org/skypilot int…

8221c35

…o init_container_fix

romilbhardwaj requested a review from Michaelvll July 24, 2024 21:30

Michaelvll reviewed Jul 24, 2024

View reviewed changes

fix comments

fd66d03

romilbhardwaj requested a review from Michaelvll July 24, 2024 22:08

Michaelvll approved these changes Jul 24, 2024

View reviewed changes

fix comments

0d45a11

romilbhardwaj added this pull request to the merge queue Jul 24, 2024

Merged via the queue into master with commit 754bf57 Jul 24, 2024
20 checks passed

romilbhardwaj deleted the init_container_fix branch July 24, 2024 23:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[k8s] Update waiting logic for init containers #3762

[k8s] Update waiting logic for init containers #3762

romilbhardwaj commented Jul 18, 2024

Michaelvll left a comment

Michaelvll Jul 24, 2024

romilbhardwaj Jul 24, 2024

Michaelvll Jul 24, 2024

romilbhardwaj Jul 24, 2024

Michaelvll Jul 24, 2024 •

edited

Loading

romilbhardwaj Jul 24, 2024

Michaelvll Jul 24, 2024

romilbhardwaj Jul 24, 2024

Michaelvll left a comment

Michaelvll Jul 24, 2024

Michaelvll Jul 24, 2024

romilbhardwaj commented Jul 24, 2024

		if (init_waiting is not None and init_waiting.reason
		not in ['ContainerCreating', 'PodInitializing']):

[k8s] Update waiting logic for init containers #3762

[k8s] Update waiting logic for init containers #3762

Conversation

romilbhardwaj commented Jul 18, 2024

Michaelvll left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Michaelvll Jul 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Michaelvll left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

romilbhardwaj commented Jul 24, 2024

Michaelvll Jul 24, 2024 •

edited

Loading