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

github actions: upgrade to checkout version 3 #376

Merged
merged 2 commits into from
Feb 14, 2023

Conversation

erav
Copy link
Member

@erav erav commented Feb 14, 2023

Version 2 of the checkout action produces a warning and cancels the action. Upgrading to version 3.

Change-Id: I6f80aeae400104a188615444ca3bdbbcc8817898
Signed-off-by: Eitan Raviv eraviv@redhat.com

@erav erav requested review from sandrobonazzola and tinez and removed request for tinez, mz-pdm, almusil and aesteve-rh February 14, 2023 09:56
@aesteve-rh
Copy link
Member

Where did you see the warning? I have briefly checked the latest Actions and couldn't find it.

@erav
Copy link
Member Author

erav commented Feb 14, 2023

@aesteve-rh
Copy link
Member

There is a warning for the checkout, but it does not seem to cause the Action to cancel. AFAIU seems to be the podman run command that fails:

Storing signatures
time="2023-02-12T02:50:53Z" level=error msg="Updating image \"44b33053e96b60856ac33e46acff3358db2bdac20da1b5beb0a6dcc9b7a1f0df\" (old names []) failed, deleting it"
time="2023-02-12T02:50:53Z" level=error msg="Error deleting incomplete image \"44b33053e96b60856ac33e46acff3358db2bdac20da1b5beb0a6dcc9b7a1f0df\": identifier is not an image"
Error: 2 errors occurred while pulling:
 * initializing source docker://ovirt/vdsm-network-tests-integration:centos-8: reading manifest centos-8 in docker.io/ovirt/vdsm-network-tests-integration: errors:
denied: requested access to the resource is denied
unauthorized: authentication required

 * committing the finished image: setting names [quay.io/ovirt/vdsm-network-tests-integration:centos-8] on image "44b33053e96b60856ac33e46acff3358db2bdac20da1b5beb0a6dcc9b7a1f0df": layer not known
+ CONTAINER_ID=

Version 2 of the checkout action produces a warning, upgrading to
version 3.
Also, set the jobs to not fail fast as this cancels all jobs after the
first failure.

Change-Id: I6f80aeae400104a188615444ca3bdbbcc8817898
Signed-off-by: Eitan Raviv <eraviv@redhat.com>
@erav
Copy link
Member Author

erav commented Feb 14, 2023

added fail-fast:false to network.yml . Functional tests have been having issues since a long time. We have even skipped them on centos-9. Now they are failing on centos-8 and alma-9 as well. Solving them on this PR is out of scope and would just delay it indefinitely. So let's merge this PR so the real issues can be addressed more easily

aesteve-rh
aesteve-rh previously approved these changes Feb 14, 2023
Copy link
Member

@aesteve-rh aesteve-rh left a comment

Choose a reason for hiding this comment

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

There were some concerns in the past regarding the update of the checkout action, but if keeping it outdated raises a problem, I think it is better to update it. Thanks for taking care!

@erav
Copy link
Member Author

erav commented Feb 14, 2023

@sandrobonazzola could you please have a look? Looks like checkout@v3 mutes the warnings which helps in reducing the noise.

Copy link
Member

@sandrobonazzola sandrobonazzola left a comment

Choose a reason for hiding this comment

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

LGTM, approving.
I would recomend having a look at https://github.com/oVirt/checkout-action as other repos in oVirt are using it instead

Change-Id: Icdc85ea61892be4f67b0327137498e2fb6cad7fc
Signed-off-by: Eitan Raviv <eraviv@redhat.com>
@erav
Copy link
Member Author

erav commented Feb 14, 2023

@sandrobonazzola can you please merge if you approve? I don't have permissions... thanks

@sandrobonazzola sandrobonazzola merged commit 9431f54 into oVirt:master Feb 14, 2023
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.

3 participants