Use env variable for the sleep image in case of using other image ref… #2570

abhijeet-dhumal · 2024-07-10T09:51:21Z

What type of PR is this?

What this PR does / why we need it:

In Disconnected environment, the images with tags/digests are supported but use of image digest is more preferred because of immutability, consistency and security.
The update suggested in this PR allows to pass a test image as a environment variable which allows to use the image with digest for running e2e test script in disconnected environment.
Use of environment variable helps to automate this test execution in CI/CD pipeline.

Which issue(s) this PR fixes:

Fixes #
Added provision to use environment variable for the sleep image in case of using other image reference in case of disconnected environment

Special notes for your reviewer:

Does this PR introduce a user-facing change?

NONE

linux-foundation-easycla · 2024-07-10T09:51:25Z

The committers listed above are authorized under a signed CLA.

✅ login: abhijeet-dhumal / name: Abhijeet Dhumal (6f0e6a2, d1dccbd, ae5f724)

k8s-ci-robot · 2024-07-10T09:51:30Z

Welcome @abhijeet-dhumal!

It looks like this is your first PR to kubernetes-sigs/kueue 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/kueue has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2024-07-10T09:51:30Z

Hi @abhijeet-dhumal. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

netlify · 2024-07-10T09:51:40Z

✅ Deploy Preview for kubernetes-sigs-kueue canceled.

Name	Link
🔨 Latest commit	`ae5f724`
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-sigs-kueue/deploys/66e8301dcae2e40008361e7e

trasc · 2024-07-11T06:10:50Z

My main concern with this is that the user may provide an image with a totally different behavior which has no chance to properly run the test.

In a disconnected environment, I think, you can just tag one of your local images as gcr.io/k8s-staging-perf-tests/sleep:v0.1.0 and it will be used in the e2e test.

It should be loaded into the kind cluster here

kueue/hack/e2e-common.sh

Line 47 in fc7477d

cluster_kind_load_image $1 $E2E_TEST_IMAGE

.

abhijeet-dhumal · 2024-07-11T10:28:24Z

My main concern with this is that the user may provide an image with a totally different behavior which has no chance to properly run the test.

In a disconnected environment, I think, you can just tag one of your local images as gcr.io/k8s-staging-perf-tests/sleep:v0.1.0 and it will be used in the e2e test.

Ot should be loaded into the kind cluster here

kueue/hack/e2e-common.sh

Line 47 in fc7477d

cluster_kind_load_image $1 $E2E_TEST_IMAGE

.

Hello @trasc , Actually the update suggested in this PR allows use of environment variable to pass same image but with digest which in turn helps to automate this test execution in CI/CD pipeline for disconnected environment.

Thank you for reviewing !

abhijeet-dhumal · 2024-07-23T05:21:12Z

@denkensk @mimowo @kerthcet @trasc May I have your review on this PR?

trasc · 2024-07-23T05:30:35Z

@denkensk @mimowo @kerthcet @trasc May I have your review on this PR?

Hi, in my opinion you can work around this issue without any code changes, please try my proposal.

In a disconnected environment, I think, you can just tag one of your local images as gcr.io/k8s-staging-perf-tests/sleep:v0.1.0 and it will be used in the e2e test.

mimowo · 2024-07-23T08:36:25Z

@denkensk @mimowo @kerthcet @trasc May I have your review on this PR?

So, IIUC the name of the image on your env is the same, you just want to be able to add the digest due to security reasons? If this is the case, would it work for you just to have an env var to specify the hash?

abhijeet-dhumal · 2024-07-25T05:52:55Z

@denkensk @mimowo @kerthcet @trasc May I have your review on this PR?

Hi, in my opinion you can work around this issue without any code changes, please try my proposal.

In a disconnected environment, I think, you can just tag one of your local images as gcr.io/k8s-staging-perf-tests/sleep:v0.1.0 and it will be used in the e2e test.

Hi @trasc, I greatly value your suggestion and have considered this option to make it work, but it's not feasible!

abhijeet-dhumal · 2024-07-25T05:57:23Z

@denkensk @mimowo @kerthcet @trasc May I have your review on this PR?

So, IIUC the name of the image on your env is the same, you just want to be able to add the digest due to security reasons? If this is the case, would it work for you just to have an env var to specify the hash?

@mimowo Thank you for your suggestion, I think it may work, and i will proceed with updating this PR accordingly!

mimowo · 2024-07-25T06:39:36Z

One more question trying to get to the bottom of your use case.

if you just need the hash to be present on the running image for the pod, then instead of supplying the image as env var would it work for you to read the dijest during the build? For example using docker inspect in bash and supplying it to the runner, or reading inside the runner.

I think passing the dijest might bring overhead to maintenance for you if we have more testing images.

trasc · 2024-07-25T07:09:47Z

Hi @trasc, I greatly value your suggestion and have considered this option to make it work, but it's not feasible!

Can you elaborate on this?

test/e2e/singlecluster/environment.go

trasc · 2024-07-25T07:23:14Z

/ok-to-test

hack/e2e-common.sh

site/static/examples/python/sample-job.py

test/util/e2e.go

hack/e2e-common.sh

hack/e2e-test.sh

mbobrovskyi · 2024-09-13T05:11:09Z

hack/multikueue-e2e-test.sh

@@ -20,7 +20,6 @@ set -o pipefail

 SOURCE_DIR="$(cd "$(dirname -- "${BASH_SOURCE[0]}")" && pwd -P)"
 ROOT_DIR="$SOURCE_DIR/.."
-export E2E_TEST_IMAGE=gcr.io/k8s-staging-perf-tests/sleep:v0.1.0


And here also. On this way try to use my previous suggestion #2570 (comment). It should work for both cases.

Hey @mbobrovskyi I have updated tests accordingly but not sure why PR check e2e tests are failing 🤔
Could you please guide me in what is the issue here ?

Yes, this is a good question. With tag name and digest I can see an error Error: failed to get image from containerd "sha256:d483aa6092983e8adea8d92d5661f7191f2e1b07ac480ec3a60a1584fea39a3c": image "docker.io/library/import-2024-09-13@sha256:12abe35a0f1d3da31f825e36ed20a1978ca1848d9973aa6c224fd5725ac6ebbc": not found.

kubectl describe pods test-job-1-ckbj8 -n e2e-9prhl Name: test-job-1-ckbj8 Namespace: e2e-9prhl Priority: 100 Priority Class Name: high Service Account: default Node: kind-worker2/172.18.0.2 Start Time: Fri, 13 Sep 2024 09:02:24 +0300 Labels: batch.kubernetes.io/controller-uid=3d540ff4-b19d-4aad-9b3f-4cf0af1f0ec6 batch.kubernetes.io/job-name=test-job-1 controller-uid=3d540ff4-b19d-4aad-9b3f-4cf0af1f0ec6 job-name=test-job-1 Annotations: <none> Status: Pending IP: 10.244.2.10 IPs: IP: 10.244.2.10 Controlled By: Job/test-job-1 Containers: c: Container ID: Image: gcr.io/k8s-staging-perf-tests/sleep:v0.1.0@sha256:8d91ddf9f145b66475efda1a1b52269be542292891b5de2a7fad944052bab6ea Image ID: Port: <none> Host Port: <none> Args: 1m State: Waiting Reason: CreateContainerError Ready: False Restart Count: 0 Requests: cpu: 1 Environment: <none> Mounts: /var/run/secrets/kubernetes.io/serviceaccount from kube-api-access-f6424 (ro) Conditions: Type Status PodReadyToStartContainers True Initialized True Ready False ContainersReady False PodScheduled True Volumes: kube-api-access-f6424: Type: Projected (a volume that contains injected data from multiple sources) TokenExpirationSeconds: 3607 ConfigMapName: kube-root-ca.crt ConfigMapOptional: <nil> DownwardAPI: true QoS Class: Burstable Node-Selectors: <none> Tolerations: node.kubernetes.io/not-ready:NoExecute op=Exists for 300s node.kubernetes.io/unreachable:NoExecute op=Exists for 300s Events: Type Reason Age From Message ---- ------ ---- ---- ------- Normal Scheduled 27s default-scheduler Successfully assigned e2e-9prhl/test-job-1-ckbj8 to kind-worker2 Normal Pulled 3s (x4 over 27s) kubelet Container image "gcr.io/k8s-staging-perf-tests/sleep:v0.1.0@sha256:8d91ddf9f145b66475efda1a1b52269be542292891b5de2a7fad944052bab6ea" already present on machine Warning Failed 3s (x4 over 27s) kubelet Error: failed to get image from containerd "sha256:d483aa6092983e8adea8d92d5661f7191f2e1b07ac480ec3a60a1584fea39a3c": image "docker.io/library/import-2024-09-13@sha256:12abe35a0f1d3da31f825e36ed20a1978ca1848d9973aa6c224fd5725ac6ebbc": not found

As I can see here: Image name with tag and digest. Only digest will be used for pulling. So no difference.

So even if we keep image with tag+digest it will work, then wondering why the PR checks are failing 👀

I also tested this solution locally with removing images, so it should work fine.

I like the proposal, just a small question:

# $1 cluster function cluster_kind_load { e2e_test_sleep_image_without_sha=${E2E_TEST_SLEEP_IMAGE%%@*} # ... docker tag $E2E_TEST_SLEEP_IMAGE "$e2e_test_sleep_image_without_sha" cluster_kind_load_image "$1" "${e2e_test_sleep_image_without_sha}" cluster_kind_load_image "$1" "$IMAGE_TAG" }

would it work @mbobrovskyi ? Seems simpler as there are fewer steps.

Yeap, it works.

@abhijeet-dhumal could you please apply this changes?

Thank you both @mbobrovskyi @mimowo !
I truely appreciate your dedicated efforts toward enhancing this PR. 😊

k8s-ci-robot · 2024-09-13T05:43:50Z

@abhijeet-dhumal: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-kueue-test-kjobctl	`2b38d66`	link	true	`/test pull-kueue-test-kjobctl`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

…e reference or same image but with digest in case of disconnected environment

hack/e2e-common.sh

…ch is already pulled. For more information : kubernetes-sigs/kind#2394

mbobrovskyi · 2024-09-16T13:19:36Z

/lgtm

Thanks!

k8s-ci-robot · 2024-09-16T13:19:42Z

LGTM label has been added.

Git tree hash: fed76e50dbf74951f6674daf7b729bcab03f9655

abhijeet-dhumal · 2024-09-16T13:40:37Z

@mbobrovskyi @mimowo @alculquicondor @trasc @apodhrad @denkensk @kerthcet @tenzen-y
Thank you all !!
Your continued contributions have been a great help in improving this PR. I truly appreciate it ! 😊

mimowo · 2024-09-16T14:22:44Z

/hold for Aldo's comment.

/hold cancel
The comment is addressed along with many other comments asked since then.

mimowo · 2024-09-16T14:24:29Z

/approve
Thank you for the effort and for not getting discouraged by the back and forth before concluding on the final approach!

k8s-ci-robot · 2024-09-16T14:24:38Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhijeet-dhumal, mimowo

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [mimowo]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

kubernetes-sigs#2570) * Use env variable for the test sleep image in case of using other image reference or same image but with digest in case of disconnected environment * Update E2e test sleep image to use image digest instead of version tag * Incase of kind cluster, manually create tag for image with digest which is already pulled. For more information : kubernetes-sigs/kind#2394

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Jul 10, 2024

k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Jul 10, 2024

k8s-ci-robot requested review from denkensk and mimowo July 10, 2024 09:51

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 10, 2024

abhijeet-dhumal marked this pull request as ready for review July 10, 2024 15:58

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 10, 2024

k8s-ci-robot requested a review from kerthcet July 10, 2024 15:58

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Jul 11, 2024

trasc reviewed Jul 25, 2024

View reviewed changes

test/e2e/singlecluster/environment.go Outdated Show resolved Hide resolved

test/e2e/singlecluster/environment.go Outdated Show resolved Hide resolved

mbobrovskyi reviewed Sep 12, 2024

View reviewed changes

hack/e2e-common.sh Show resolved Hide resolved

site/static/examples/python/sample-job.py Outdated Show resolved Hide resolved

test/util/e2e.go Show resolved Hide resolved

test/util/e2e.go Outdated Show resolved Hide resolved

hack/e2e-common.sh Show resolved Hide resolved

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 12, 2024

abhijeet-dhumal force-pushed the add-sleep-img-env-var branch from dc1b50b to f5ddc57 Compare September 12, 2024 14:49

mbobrovskyi reviewed Sep 12, 2024

View reviewed changes

hack/e2e-test.sh Show resolved Hide resolved

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 13, 2024

abhijeet-dhumal force-pushed the add-sleep-img-env-var branch from f5ddc57 to 1a8b4f5 Compare September 13, 2024 05:01

mbobrovskyi reviewed Sep 13, 2024

View reviewed changes

abhijeet-dhumal force-pushed the add-sleep-img-env-var branch from 1a8b4f5 to 6b01e9a Compare September 13, 2024 05:26

abhijeet-dhumal added 2 commits September 16, 2024 17:53

Use env variable for the test sleep image in case of using other imag…

d1dccbd

…e reference or same image but with digest in case of disconnected environment

Update E2e test sleep image to use image digest instead of version tag

6f0e6a2

abhijeet-dhumal force-pushed the add-sleep-img-env-var branch from 6b01e9a to 9ab8ab8 Compare September 16, 2024 12:23

mbobrovskyi reviewed Sep 16, 2024

View reviewed changes

hack/e2e-common.sh Outdated Show resolved Hide resolved

Incase of kind cluster, manually create tag for image with digest whi…

ae5f724

…ch is already pulled. For more information : kubernetes-sigs/kind#2394

abhijeet-dhumal force-pushed the add-sleep-img-env-var branch from 9ab8ab8 to ae5f724 Compare September 16, 2024 13:18

k8s-ci-robot assigned mbobrovskyi Sep 16, 2024

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 16, 2024

abhijeet-dhumal requested a review from mbobrovskyi September 16, 2024 13:41

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 16, 2024

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 16, 2024

k8s-ci-robot merged commit 217575e into kubernetes-sigs:main Sep 16, 2024
16 checks passed

k8s-ci-robot added this to the v0.9 milestone Sep 16, 2024

tenzen-y mentioned this pull request Sep 27, 2024

Add mbobrovskyi to the top level reviewer #3156

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use env variable for the sleep image in case of using other image ref… #2570

Use env variable for the sleep image in case of using other image ref… #2570

abhijeet-dhumal commented Jul 10, 2024 •

edited by k8s-ci-robot

Loading

linux-foundation-easycla bot commented Jul 10, 2024 •

edited

Loading

k8s-ci-robot commented Jul 10, 2024

k8s-ci-robot commented Jul 10, 2024

netlify bot commented Jul 10, 2024 •

edited

Loading

trasc commented Jul 11, 2024 •

edited

Loading

abhijeet-dhumal commented Jul 11, 2024

abhijeet-dhumal commented Jul 23, 2024

trasc commented Jul 23, 2024

mimowo commented Jul 23, 2024

abhijeet-dhumal commented Jul 25, 2024

abhijeet-dhumal commented Jul 25, 2024

mimowo commented Jul 25, 2024

trasc commented Jul 25, 2024

trasc commented Jul 25, 2024

mbobrovskyi Sep 13, 2024

abhijeet-dhumal Sep 13, 2024 •

edited

Loading

mbobrovskyi Sep 13, 2024 •

edited

Loading

mbobrovskyi Sep 13, 2024

abhijeet-dhumal Sep 13, 2024

mbobrovskyi Sep 13, 2024

mimowo Sep 13, 2024

mbobrovskyi Sep 13, 2024

mbobrovskyi Sep 16, 2024

abhijeet-dhumal Sep 16, 2024

k8s-ci-robot commented Sep 13, 2024 •

edited

Loading

mbobrovskyi commented Sep 16, 2024

k8s-ci-robot commented Sep 16, 2024

abhijeet-dhumal commented Sep 16, 2024

mimowo commented Sep 16, 2024

mimowo commented Sep 16, 2024

k8s-ci-robot commented Sep 16, 2024

Use env variable for the sleep image in case of using other image ref… #2570

Use env variable for the sleep image in case of using other image ref… #2570

Conversation

abhijeet-dhumal commented Jul 10, 2024 • edited by k8s-ci-robot Loading

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

linux-foundation-easycla bot commented Jul 10, 2024 • edited Loading

k8s-ci-robot commented Jul 10, 2024

k8s-ci-robot commented Jul 10, 2024

netlify bot commented Jul 10, 2024 • edited Loading

✅ Deploy Preview for kubernetes-sigs-kueue canceled.

trasc commented Jul 11, 2024 • edited Loading

abhijeet-dhumal commented Jul 11, 2024

abhijeet-dhumal commented Jul 23, 2024

trasc commented Jul 23, 2024

mimowo commented Jul 23, 2024

abhijeet-dhumal commented Jul 25, 2024

abhijeet-dhumal commented Jul 25, 2024

mimowo commented Jul 25, 2024

trasc commented Jul 25, 2024

trasc commented Jul 25, 2024

Choose a reason for hiding this comment

abhijeet-dhumal Sep 13, 2024 • edited Loading

Choose a reason for hiding this comment

mbobrovskyi Sep 13, 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

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

k8s-ci-robot commented Sep 13, 2024 • edited Loading

mbobrovskyi commented Sep 16, 2024

k8s-ci-robot commented Sep 16, 2024

abhijeet-dhumal commented Sep 16, 2024

mimowo commented Sep 16, 2024

mimowo commented Sep 16, 2024

k8s-ci-robot commented Sep 16, 2024

abhijeet-dhumal commented Jul 10, 2024 •

edited by k8s-ci-robot

Loading

linux-foundation-easycla bot commented Jul 10, 2024 •

edited

Loading

netlify bot commented Jul 10, 2024 •

edited

Loading

trasc commented Jul 11, 2024 •

edited

Loading

abhijeet-dhumal Sep 13, 2024 •

edited

Loading

mbobrovskyi Sep 13, 2024 •

edited

Loading

k8s-ci-robot commented Sep 13, 2024 •

edited

Loading