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

Test image has incorrect tag? #1329

Closed
Jeffwan opened this issue Aug 3, 2021 · 5 comments · Fixed by #1334
Closed

Test image has incorrect tag? #1329

Jeffwan opened this issue Aug 3, 2021 · 5 comments · Fixed by #1334

Comments

@Jeffwan
Copy link
Member

Jeffwan commented Aug 3, 2021

https://argo.kubeflow-testing.com/workflows/kubeflow-test-infra/kubeflow-tf-operator-presubmit-v1-1325-8430a04-8224-a787?tab=workflow&nodeId=kubeflow-tf-operator-presubmit-v1-1325-8430a04-8224-a787-1831026653

Kaniko use $(PULL_BASE_SHA) as image tag. Is this expected?

Should we use PULL_PULL_SHA which is the head? Luckily, this doesn't affect real test because setup-tf-operator.sh uses same tag.
https://github.com/kubeflow/tf-operator/blob/6f1e96c4b3e7b63860780b6f572ec229e58ca146/scripts/setup-tf-operator.sh#L28

/kaniko/executor --dockerfile=/mnt/test-data-volume/kubeflow-tf-operator-presubmit-v1-1325-8430a04-8224-a787/src/kubeflow/tf-operator/build/images/training-operator/Dockerfile --context=dir:///mnt/test-data-volume/kubeflow-tf-operator-presubmit-v1-1325-8430a04-8224-a787/src/kubeflow/tf-operator --destination=$(ECR_REGISTRY):$(PULL_BASE_SHA)

image

Kaniko logs

INFO[0112] WORKDIR /
INFO[0112] cmd: workdir
INFO[0112] Changed working directory to /
INFO[0112] No files changed in this command, skipping snapshotting.
INFO[0112] COPY --from=builder /workspace/manager .
INFO[0112] Taking snapshot of files...
INFO[0113] ENTRYPOINT ["/manager"]
INFO[0113] Pushing image to 809251082950.dkr.ecr.us-west-2.amazonaws.com/tf-operator:43d950df3b75271ae2b77ef5a7c291c4c21cf7d8
INFO[0115] Pushed image to 1 destinations

/cc @PatrickXYS

@PatrickXYS
Copy link
Member

Yeah I think it's better we can use $PULL_PULL_SHA, we should make a one-line change https://github.com/kubeflow/tf-operator/blob/6f1e96c4b3e7b63860780b6f572ec229e58ca146/scripts/setup-tf-operator.sh#L28 to and it should work.

@Jeffwan
Copy link
Member Author

Jeffwan commented Aug 5, 2021

/assign @PatrickXYS

Can you help file a PR?

@Jeffwan
Copy link
Member Author

Jeffwan commented Aug 6, 2021

I didn't review carefully. We should change kaniko side as well.

a5bdae7

This commit fully resolve the issue

@Jeffwan Jeffwan reopened this Aug 6, 2021
@Jeffwan
Copy link
Member Author

Jeffwan commented Aug 6, 2021

I reopen the issue. The major issue is post submit job doesn't have PULL_PULL_SHA. Above changes will fail the pipeline.

We need to use PULL_PULL_SHA for presubmit Jobs and PULL_BASE_SHA for postsubmit jobs.

Note: Even we use PULL_BASE_SHA, it's fine. because private images is not exposed to end users. Tag is incorrect but it's consistent in the test and we still use the right Dockerfile. So this is a p2 issue

/priority p2

@stale
Copy link

stale bot commented Mar 2, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the lifecycle/stale label Mar 2, 2022
@stale stale bot closed this as completed Apr 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants