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

Improve Pod Container Status Handling #356

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Bianco95
Copy link
Collaborator

Description

Updated handling of container statuses and introduces better support for init containers in the system.

Key Changes

  1. Preserve Failed Container Status

    • Successful containers no longer overwrite the status of failed containers, ensuring accurate state reporting.
  2. Improved Init Container Handling

    • If a pod includes init containers, the system now verifies their completion before proceeding to the main containers.
    • On the plugin side, init containers should be correctly handled:
      • Executed sequentially in the defined order.
      • Must complete before starting the main containers.

These changes ensure a more robust and predictable behavior for pods with complex container setups, aligning the implementation with expected Kubernetes standards.

Testing

To validate the new changes, testing was conducted using a setup that includes a Virtual Kubelet (VK) deployed in a Kubernetes (k8s) cluster created with kind, an InterLink API server running behind an OAuth proxy on an edge node alongside the Docker plugin.

  1. Tests from the suite available in this repo https://github.com/interTwin-eu/vk-test-set have been successful executed.
========================================================================== test session starts ==========================================================================
platform linux -- Python 3.10.12, pytest-8.2.0, pluggy-1.5.0 -- /usr/bin/python3
cachedir: .pytest_cache
rootdir: /home/giulio/vk-setup/vk-test-set
configfile: pyproject.toml
plugins: typeguard-4.3.0
collected 10 items                                                                                                                                                      

vktestset/basic_test.py::test_namespace_exists[interlink] PASSED                                                                                                  [ 10%]
vktestset/basic_test.py::test_node_exists[virtual-node] PASSED                                                                                                    [ 20%]
vktestset/basic_test.py::test_manifest[virtual-node-000-hello-world.yaml] PASSED                                                                                  [ 30%]
vktestset/basic_test.py::test_manifest[virtual-node-010-simple-python.yaml] PASSED                                                                                [ 40%]
vktestset/basic_test.py::test_manifest[virtual-node-020-python-env.yaml] PASSED                                                                                   [ 50%]
vktestset/basic_test.py::test_manifest[virtual-node-030-simple-shared-volume.yaml] PASSED                                                                         [ 60%]
vktestset/basic_test.py::test_manifest[virtual-node-040-config-volumes.yaml] PASSED                                                                               [ 70%]
vktestset/basic_test.py::test_manifest[virtual-node-050-limits.yaml] PASSED                                                                                       [ 80%]
vktestset/basic_test.py::test_manifest[virtual-node-060-init-container.yaml] PASSED                                                                               [ 90%]
vktestset/basic_test.py::test_manifest[virtual-node-070-rclone-bind.yaml] PASSED                                                                                  [100%]

==================================================================== 10 passed in 120.25s (0:02:00) =====================================================================
  1. A POD with multiple init containers and multiple containers has been tested by also varying the order of the failed and successful containers
apiVersion: v1
kind: Pod
metadata:
  name: hello-world-1
  namespace: interlink

spec:
  restartPolicy: Never
  nodeSelector:
    kubernetes.io/hostname: virtual-node

  initContainers:
  - name: init-cnt1
    image: ghcr.io/grycap/cowsay
    command: ["/bin/sh", "-c"]
    args: ["echo 'init cnt 1. now sleep'; sleep 5; exit 0"]
    imagePullPolicy: Always

  - name: init-cnt2
    image: ghcr.io/grycap/cowsay
    command: ["/bin/sh", "-c"]
    args: ["echo 'init cnt 2. now sleep'; sleep 5; exit 0"]
    imagePullPolicy: Always

  containers:
  - name: successful-container
    image: busybox
    command: ["/bin/sh", "-c"]
    args: ["exit 0"]
    imagePullPolicy: Always

  - name: hello-world
    image: ghcr.io/grycap/cowsay
    command: ["/bin/sh", "-c"]
    args: ["echo 'hello world'; sleep 5"]
    imagePullPolicy: Always

  - name: fail-container
    image: busybox
    command: ["/bin/sh", "-c"]
    args: ["exit 1"]  # FORCE FAIL EXIT CODE
    imagePullPolicy: Always

  dnsPolicy: ClusterFirst

  tolerations:
    - key: virtual-node.interlink/no-schedule
      operator: Exists
      effect: NoSchedule

…dded logic to ensure that successful containers no longer overwrite failed containers, preserving the correct status. - Improved handling of init containers: - If a pod has init containers, the pod's status now checks their completion first. - On the plugin side, init containers must be executed correctly: they must run sequentially and complete before the main containers.
Copy link
Collaborator

@dciangot dciangot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Bianco95 lgtm, can you also open an issue for vktest repo noting that we need to include tests for this scenarios (your point 2)? That way we have the test enabled in the e2e PR check

@Bianco95
Copy link
Collaborator Author

Ok, an issue has been opened in the vk-test-set repo (interTwin-eu/vk-test-set#7)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Successful containers should not overwrite Failed phase set by former containers
2 participants