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.

Sign up for GitHub

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

Jump to bottom

[Fix] update bash scripts for E2E tests run to propagate failures #433

Merged
merged 2 commits into from
Nov 25, 2024
Merged

[Fix] update bash scripts for E2E tests run to propagate failures #433

merged 2 commits into from
Nov 25, 2024

Conversation

jstourac
Copy link
Member

@jstourac jstourac commented Oct 25, 2024 •
edited
Loading

We haven't been propagating the test failures in our E2E CI jobs because we weren't checking for the return codes of the commands run in the bash scripts.

Also, this fixes setup* targets in Makefile so that expected images are deployed to the testing cluster.

This depends on #432!

I bumped into this during my work on https://issues.redhat.com/browse/RHOAIENG-14687.

This fix is part of https://issues.redhat.com/browse/RHOAIENG-15141.

Description

How Has This Been Tested?

Try to run in components/odh-notebook-controller:

  • make run-ci-e2e-tests - 15 pass, 2 fail - you need to be logged into some cluster to make this work
  • since 2 tests are failing, see that now the execution itself fails with return code 2 - before this change the return code was just 0 as a success, which was wrong of course

Merge criteria:

  • 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

@jstourac jstourac self-assigned this Oct 25, 2024
@jstourac jstourac marked this pull request as draft October 25, 2024 19:54
@jstourac jstourac mentioned this pull request Oct 25, 2024
Merged
3 tasks
@jiridanek
Copy link
Member

/retest ci/prow/odh-notebook-controller-e2e

@openshift-ci OpenShift CI
Copy link

openshift-ci bot commented Nov 8, 2024

@jiridanek: The /retest command does not accept any targets.
The following commands are available to trigger required jobs:

  • /test images
  • /test kf-notebook-controller-pr-image-mirror
  • /test kf-notebook-controller-unit
  • /test odh-notebook-controller-e2e
  • /test odh-notebook-controller-pr-image-mirror
  • /test odh-notebook-controller-unit

Use /test all to run the following jobs that were automatically triggered:

  • pull-ci-opendatahub-io-kubeflow-main-images
  • pull-ci-opendatahub-io-kubeflow-main-kf-notebook-controller-pr-image-mirror
  • pull-ci-opendatahub-io-kubeflow-main-odh-notebook-controller-e2e
  • pull-ci-opendatahub-io-kubeflow-main-odh-notebook-controller-pr-image-mirror
  • pull-ci-opendatahub-io-kubeflow-main-odh-notebook-controller-unit

In response to this:

/retest ci/prow/odh-notebook-controller-e2e

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 jiridanek marked this pull request as ready for review November 8, 2024 17:31
@jiridanek jiridanek marked this pull request as draft November 8, 2024 17:31
@jstourac jstourac marked this pull request as ready for review November 8, 2024 19:38
@openshift-ci openshift-ci bot requested a review from jiridanek November 8, 2024 19:38
@jiridanek
Copy link
Member

/lgtm
When it's broken, then lets make it visible!

@openshift-ci openshift-ci bot added the lgtm label Nov 12, 2024
@atheo89
Copy link
Member

atheo89 commented Nov 14, 2024

/test odh-notebook-controller-e2e
/lgtm

@jstourac jstourac force-pushed the fixE2eReturnCodePropagation branch 2 times, most recently from 4ff2f2a to 33845b5 Compare November 21, 2024 10:39
@openshift-ci openshift-ci bot removed the lgtm label Nov 21, 2024
@jstourac
Copy link
Member Author

/retest ci/prow/odh-notebook-controller-e2e ci/prow/odh-notebook-controller-unit

@jstourac
[Fix] update bash scripts for E2E tests run to propagate failures
366dda5 
We haven't been propagating the test failures in our E2E CI jobs because
we weren't checking for the return codes of the commands run in the bash
scripts.

Also, this fixes setup* targets in Makefile so that expected images are
deployed to the testing cluster.
@openshift-ci OpenShift CI
Copy link

openshift-ci bot commented Nov 21, 2024

@jstourac: No presubmit jobs available for opendatahub-io/kubeflow@main

In response to this:

/retest ci/prow/odh-notebook-controller-e2e ci/prow/odh-notebook-controller-unit

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Nov 24, 2024
@jstourac
Copy link
Member Author

/retest ci/prow/odh-notebook-controller-e2e

@openshift-ci OpenShift CI
Copy link

openshift-ci bot commented Nov 25, 2024

@jstourac: The /retest command does not accept any targets.
The following commands are available to trigger required jobs:

/test images

/test kf-notebook-controller-pr-image-mirror

/test kf-notebook-controller-unit

/test odh-notebook-controller-e2e

/test odh-notebook-controller-pr-image-mirror

/test odh-notebook-controller-unit

Use /test all to run the following jobs that were automatically triggered:

pull-ci-opendatahub-io-kubeflow-main-images

pull-ci-opendatahub-io-kubeflow-main-kf-notebook-controller-pr-image-mirror

pull-ci-opendatahub-io-kubeflow-main-odh-notebook-controller-e2e

pull-ci-opendatahub-io-kubeflow-main-odh-notebook-controller-pr-image-mirror

pull-ci-opendatahub-io-kubeflow-main-odh-notebook-controller-unit

In response to this:

/retest ci/prow/odh-notebook-controller-e2e

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
Copy link
Member Author

/test odh-notebook-controller-e2e

@jstourac
Copy link
Member Author

So, finally - we can see the tests to be executed. There are two tests failing on us right now. But these will be fixed in a separate PR.

Thank you for your review, Jiri!

/override ci/prow/odh-notebook-controller-e2e
/override ci/prow/images
/override /ci/prow/kf-notebook-controller-pr-image-mirror
/override /ci/prow/odh-notebook-controller-pr-image-mirror
/approve

@openshift-ci OpenShift CI
Copy link

openshift-ci bot commented Nov 25, 2024

@jstourac: /override requires failed status contexts, check run or a prowjob name to operate on.
The following unknown contexts/checkruns were given:

  • /ci/prow/kf-notebook-controller-pr-image-mirror
  • /ci/prow/odh-notebook-controller-pr-image-mirror

Only the following failed contexts/checkruns were expected:

  • ci/prow/images
  • ci/prow/kf-notebook-controller-pr-image-mirror
  • ci/prow/odh-notebook-controller-e2e
  • ci/prow/odh-notebook-controller-pr-image-mirror
  • pull-ci-opendatahub-io-kubeflow-main-odh-notebook-controller-e2e
  • tide

If you are trying to override a checkrun that has a space in it, you must put a double quote on the context.

In response to this:

So, finally - we can see the tests to be executed. There are two tests failing on us right now. But these will be fixed in a separate PR.

Thank you for your review, Jiri!

/override ci/prow/odh-notebook-controller-e2e
/override ci/prow/images
/override /ci/prow/kf-notebook-controller-pr-image-mirror
/override /ci/prow/odh-notebook-controller-pr-image-mirror
/approve

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-ci OpenShift CI
Copy link

openshift-ci bot commented Nov 25, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jstourac

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

/override ci/prow/kf-notebook-controller-pr-image-mirror
/override ci/prow/odh-notebook-controller-pr-image-mirror

@openshift-ci OpenShift CI
Copy link

openshift-ci bot commented Nov 25, 2024

@jstourac: Overrode contexts on behalf of jstourac: ci/prow/kf-notebook-controller-pr-image-mirror, ci/prow/odh-notebook-controller-pr-image-mirror

In response to this:

/override ci/prow/kf-notebook-controller-pr-image-mirror
/override ci/prow/odh-notebook-controller-pr-image-mirror

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
Copy link
Member Author

/override ci/prow/images
/override ci/prow/odh-notebook-controller-e2e

@openshift-ci OpenShift CI
Copy link

openshift-ci bot commented Nov 25, 2024

@jstourac: Overrode contexts on behalf of jstourac: ci/prow/images, ci/prow/odh-notebook-controller-e2e

In response to this:

/override ci/prow/images
/override ci/prow/odh-notebook-controller-e2e

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 9294927 into opendatahub-io:main Nov 25, 2024
12 checks passed
@jstourac jstourac deleted the fixE2eReturnCodePropagation branch November 25, 2024 14:06
This was referenced Nov 25, 2024
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jiridanek jiridanek jiridanek left review comments

@harshad16 harshad16 Awaiting requested review from harshad16

@paulovmr paulovmr Awaiting requested review from paulovmr

@atheo89 atheo89 Awaiting requested review from atheo89

@caponetto caponetto Awaiting requested review from caponetto

Assignees

@jiridanek jiridanek

@jstourac jstourac

@atheo89 atheo89

Labels
approved lgtm tide/merge-method-squash
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jstourac @jiridanek @atheo89