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

[CI] Enhance params env check script #567

Merged

Conversation

jstourac
Copy link
Member

@jstourac jstourac commented Jun 15, 2024

  • [CI] let's run params-env workflow also on push
  • [CI] enhance the check-params-env.sh to also check uniqueness of values
  • [CI] check-params-env.sh prints also time of creation of the checked image

More info as description of each commit.

https://issues.redhat.com/browse/RHOAIENG-8812

Description

How Has This Been Tested?

In the root of the repository, run:

  • ./ci/check-params-env.sh
  • ./ci/check-runtime-images.sh

We should backport this to relevant branches upstream and downstream.

  • The commits are squashed in a cohesive manner and have meaningful messages.
  • Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has manually tested the changes and verified that the changes work

Copy link
Contributor

openshift-ci bot commented Jun 15, 2024

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@jstourac
Copy link
Member Author

This is expected - as it will be fixed by #565.

Let's run the params-env workflow that checks values in params.env
and commit.env files also on push event and also on dispatch_workflow.
Up to now, it only checked that variables used in params.env file are
unique. This change checks also that the images referenced are unique as
we don't expect any of the given variables to hold the same reference.
@jstourac jstourac self-assigned this Jun 18, 2024
@jstourac jstourac marked this pull request as ready for review June 18, 2024 19:34
@openshift-ci openshift-ci bot requested review from dibryant and paulovmr June 18, 2024 19:34
@jstourac jstourac requested a review from jiridanek June 18, 2024 19:34
@jstourac
Copy link
Member Author

Technically this one is ready for review. I just need to think whether we shall squash it to a one commit for easier cherry-picking to other branches or not 🙃 🙂

@jiridanek
Copy link
Member

We can always implement the tide/merge-method-squash girlthub label, and then squashing/not squashing can be done by tide, and won't require force-push to PR by author.

Copy link
Member

@jiridanek jiridanek left a comment

Choose a reason for hiding this comment

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

I wouldn't do this in bash... But it's already done in bash and it checks useful things.

/lgtm

@jstourac
Copy link
Member Author

I wouldn't do this in bash... But it's already done in bash and it checks useful things.

What would you use instead of bash?

@jiridanek
Copy link
Member

Python; I think there is enough "logic" in the script for that. But ad I said, since it's in bash already, I'm fine with it.

@jstourac
Copy link
Member Author

Since I don't change any functional code, for the record and as an exercise - review the test failures here:

  1. ci/prow/images:
: Build image runtime-cuda-tensorflow-ubi8-python-3.8 from the repository 
{  error occurred handling build runtime-cuda-tensorflow-ubi8-python-3.8-amd64: could not get build runtime-cuda-tensorflow-ubi8-python-3.8-amd64: builds.build.openshift.io "runtime-cuda-tensorflow-ubi8-python-3.8-amd64" not found}

In build log, see:

Pushing image image-registry.openshift-image-registry.svc:5000/ci-op-1ks48214/pipeline:runtime-cuda-tensorflow-ubi8-python-3.8-amd64 ...
Getting image source signatures
Copying blob sha256:7822e944d15c45e998e88e0638073a1974246aea8fd268a925948eb2e070e048
Copying blob sha256:b7029bcb1edaf3277934b59207226ddf843bfbb348a24e5c7e84080488fa35a7
Copying blob sha256:8756f22094d074e5ea7b13b5a7cb8c5132b61a8b39d550f58e6a6053e4b3530d
Copying blob sha256:bea2a0b08f4fd7df72285c8ccf71ff0e9b76c025a0bc4dc67a4f40695feb0eca
Copying blob sha256:b82ddf37e40febb44c258077df217aef2b72f65c2c190ecd3a165ae894256e11
Copying blob sha256:f534e68b04663ce568284dc947b748f2c1793baaecb9573fd8f51a5ae25f1b1b
Copying blob sha256:21c47f62671b8ef160e5748e575f9a5c4642c7b1225c67b9ea7fc0d3417d4747
Error: received unexpected terminate signal

So the build failed at the point when it was being pushed to the image registry.

  1. ci/prow/notebooks-e2e-tests:
: Run multi-stage test notebooks-e2e-tests - notebooks-e2e-tests-jupyter-intel-tf-ubi9-python-3.9-test-e2e container test

In test build log:

bin/kubectl port-forward svc/jupyter-intel-tensorflow-ubi9-python-3-9-notebook 8888:8888 & curl --retry 5 --retry-delay 5 --retry-connrefused http://localhost:8888/notebook/opendatahub/jovyan/api ; EXIT_CODE=$?; echo && pkill --full "^bin/kubectl.*port-forward.*"; \

  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed

  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0Warning: Transient problem: connection refused Will retry in 5 seconds. 5 
Warning: retries left.
Forwarding from 127.0.0.1:8888 -> 8888
Forwarding from [::1]:8888 -> 8888

  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0Handling connection for 8888
E0618 22:42:48.486331      84 portforward.go:406] an error occurred forwarding 8888 -> 8888: error forwarding port 8888 to pod ea2566031bc9bfff803ae4f828369012152c6baabd2e85b9b57d28ed32768bda, uid : port forward into network namespace "/var/run/netns/71e5135a-8375-4d71-ac48-bd1227bebf53": failed to connect to localhost:8888 inside namespace ea2566031bc9bfff803ae4f828369012152c6baabd2e85b9b57d28ed32768bda: dial tcp [::1]:8888: connect: connection refused

  0     0    0     0    0     0      0      0 --:E0618 22:42:48.486853      84 portforward.go:234] lost connection to pod
--:-- --:--:-- --:--:--     0
curl: (52) Empty reply from server

Looks like the expected service isn't listening on the image at expected URL and port.

: Run multi-stage test test phase

Two tests failed - one is same as above, the second one is related to the previous one:

{"component":"entrypoint","error":"wrapped process failed: exit status 2","file":"sigs.k8s.io/prow/pkg/entrypoint/run.go:84","func":"sigs.k8s.io/prow/pkg/entrypoint.Options.internalRun","level":"error","msg":"Error executing test process","severity":"error","time":"2024-06-18T22:42:48Z"}
error: failed to execute wrapped command: exit status 2

Copy link
Contributor

openshift-ci bot commented Jun 19, 2024

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by: jiridanek

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:

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

@jstourac
Copy link
Member Author

Python; I think there is enough "logic" in the script for that. But ad I said, since it's in bash already, I'm fine with it.

Yeah, truth is that this is still 100times better readable for me and easier to maintain than a Python script; but I agree, it's just my problem. We can rewrite this to Python someday if we feel so.

@jiridanek
Copy link
Member

Nah, would not rewrite, not worth it.

@atheo89
Copy link
Member

atheo89 commented Jun 19, 2024

I will override the tests since are not related with the pr content, to continue with the merging.
Thanks Jan for this work!

/override ci/prow/notebooks-e2e-tests
/override ci/prow/images

Copy link
Contributor

openshift-ci bot commented Jun 19, 2024

@atheo89: Overrode contexts on behalf of atheo89: ci/prow/images, ci/prow/notebooks-e2e-tests

In response to this:

I will override the tests since are not related with the pr content, to continue with the merging.
Thanks Jan for this work!

/override ci/prow/notebooks-e2e-tests
/override ci/prow/images

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.

@openshift-merge-bot openshift-merge-bot bot merged commit 8461fce into opendatahub-io:main Jun 19, 2024
34 checks passed
@jiridanek
Copy link
Member

/cherrypick 2024a 2023b 2023a

@openshift-cherrypick-robot

@jiridanek: new pull request created: #575

In response to this:

/cherrypick 2024a 2023b 2023a

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.

@jiridanek
Copy link
Member

/cherrypick 2023b 2023a

@openshift-cherrypick-robot

@jiridanek: #567 failed to apply on top of branch "2023b":

Applying: let's run params-env workflow also on push
Using index info to reconstruct a base tree...
A	.github/workflows/params-env.yaml
Falling back to patching base and 3-way merge...
CONFLICT (modify/delete): .github/workflows/params-env.yaml deleted in HEAD and modified in let's run params-env workflow also on push. Version let's run params-env workflow also on push of .github/workflows/params-env.yaml left in tree.
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 let's run params-env workflow also on push
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherrypick 2023b 2023a

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.

@jiridanek
Copy link
Member

/cherrypick 2023a

@openshift-cherrypick-robot

@jiridanek: #567 failed to apply on top of branch "2023a":

Applying: let's run params-env workflow also on push
Using index info to reconstruct a base tree...
A	.github/workflows/params-env.yaml
Falling back to patching base and 3-way merge...
CONFLICT (modify/delete): .github/workflows/params-env.yaml deleted in HEAD and modified in let's run params-env workflow also on push. Version let's run params-env workflow also on push of .github/workflows/params-env.yaml left in tree.
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 let's run params-env workflow also on push
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherrypick 2023a

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.

@jstourac jstourac deleted the enhanceParamsEnvCheck branch July 4, 2024 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants