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 PR integration testing #531

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Fix PR integration testing #531

wants to merge 7 commits into from
+68 −21

Conversation

michael-valdron
Copy link
Member

@michael-valdron michael-valdron commented Dec 24, 2024 •
edited
Loading

Description of Changes

Summarize the changes you made as part of this pull request.

To fix the current unexpected integration testing failures this PR changes include the following:

  • Adds TLS verification enablement feature for the odo test scripts and openshift integration test script. This is to address the failing PR integration testing check due to the self signed cert not being accepted by curl when testing stacks using HTTPS rather than HTTP.
  • Migrates integration test script to use the odo v3 test script instead of odo v2 test script. This resolves failures due to some of the community stacks using newer schema versions for the default stack versions than what the odo v2 client supports.

Related Issue(s)

Link the GitHub/GitLab/JIRA issues that are related to this PR.

fixes devfile/api#1663
fixes devfile/api#1664

Acceptance Criteria

  • Contributing guide

Have you read the devfile registry contributing guide and followed its instructions?

  • Test automation

Does this repository's tests pass with your changes?

  • Documentation

Does any documentation need to be updated with your changes?

  • Check Tools Provider

Have you tested the changes with existing tools, i.e. Odo, Che, Console? (See devfile registry contributing guide on how to test changes)

Tests Performed

Explain what tests you personally ran to ensure the changes are functioning as expected.

How To Test

Instructions for the reviewer on how to test your changes.

  1. Connect to an OpenShift cluster using a self signed cert
  2. Run bash .ci/openshift_integration.sh

Notes To Reviewer

Any notes you would like to include for the reviewer.

@michael-valdron michael-valdron requested a review from a team as a code owner December 24, 2024 23:12
@openshift-ci openshift-ci bot requested review from elsony and Jdubrick December 24, 2024 23:12
@michael-valdron
Copy link
Member Author

/hold

Waiting to discuss with team in the new year about alternatives before proceeding with this change.

@michael-valdron michael-valdron mentioned this pull request Dec 24, 2024
Open
thepetk
thepetk approved these changes Jan 2, 2025
View reviewed changes
Copy link
Contributor

@thepetk thepetk left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Looks good to me label Jan 2, 2025
@openshift-ci OpenShift CI
Copy link

openshift-ci bot commented Jan 2, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: michael-valdron, thepetk

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 [michael-valdron,thepetk]

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

This was referenced Jan 2, 2025
Open
Open
Open
Open
thepetk
thepetk reviewed Jan 2, 2025
View reviewed changes
.ci/openshift_integration.sh Outdated
@@ -69,4 +69,4 @@ $(realpath odo) registry delete DefaultDevfileRegistry -f
$(realpath odo) registry add TestDevfileRegistry http://$REGISTRY_HOSTNAME

# Run the devfile validation tests
ENV=openshift REGISTRY=remote tests/check_odov2.sh $(realpath odo) $YQ_PATH
ENV=openshift REGISTRY=remote ENABLE_TLS="false" tests/check_odov2.sh $(realpath odo) $YQ_PATH
Copy link
Contributor

Choose a reason for hiding this comment

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

I took a second look here. @michael-valdron out of curiosity, could we combine the two issues in the same PR (devfile/api#1664 is the other one). Meaning we could also migrate the check to odo3 here right? This way I guess the ci check will be unblocked

Copy link
Member Author

Choose a reason for hiding this comment

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

@thepetk Yeah I can add the change to odov3 testing as well to unblock the testing entirely and resolve both. I'll work on and add it today.

@openshift-ci openshift-ci bot removed the lgtm Looks good to me label Jan 2, 2025
@openshift-ci OpenShift CI
Copy link

openshift-ci bot commented Jan 2, 2025

New changes are detected. LGTM label has been removed.

@michael-valdron michael-valdron changed the title TLS enablement for check_odov2.sh and openshift_integration.sh Fix PR integration testing Jan 2, 2025
@michael-valdron michael-valdron marked this pull request as draft January 2, 2025 22:07
@michael-valdron
Copy link
Member Author

/unhold

@thepetk
Copy link
Contributor

thepetk commented Jan 3, 2025

/test v4.15-registry-test

@michael-valdron
Copy link
Member Author

/test v4.15-registry-test

@thepetk Needs ginkgo to be installed, going to setup the binary similar to odo and yq.

@thepetk
Copy link
Contributor

thepetk commented Jan 3, 2025

/test v4.15-registry-test

@thepetk Needs ginkgo to be installed, going to setup the binary similar to odo and yq.

Yeah I've figured from the output. I just wasn't sure if the check had run after you pushed your updates and I wanted to see what will be the result.

@michael-valdron michael-valdron marked this pull request as ready for review January 3, 2025 22:52
@openshift-ci openshift-ci bot requested a review from thepetk January 3, 2025 22:52
@michael-valdron
Copy link
Member Author

michael-valdron commented Jan 3, 2025 •
edited
Loading

/hold

Holding till ci/prow/v4.15-registry-test passes.

@michael-valdron
Copy link
Member Author 

/go/src/github.com/devfile/registry/odo: /lib64/libc.so.6: version `GLIBC_2.34' not found (required by /go/src/github.com/devfile/registry/odo)
/go/src/github.com/devfile/registry/odo: /lib64/libc.so.6: version `GLIBC_2.32' not found (required by /go/src/github.com/devfile/registry/odo)

These missing libraries are most likely why this check is still failing, as it all past locally for me. Will need to include these required libraries with odo.

@michael-valdron
Copy link
Member Author

/unhold

@michael-valdron michael-valdron marked this pull request as draft January 4, 2025 00:00
This was referenced Jan 8, 2025
Open
Open
@michael-valdron
tls enablement for check_odov2.sh and openshift_integration.sh
b179860 
Signed-off-by: Michael Valdron <[email protected]>
@michael-valdron
replace odov2 with odov3 under integration testing
c624b43 
Signed-off-by: Michael Valdron <[email protected]>
@michael-valdron
fix missing ginkgo and test script argument steps
ee9afdc 
Signed-off-by: Michael Valdron <[email protected]>
@michael-valdron
correct ENABLE_TLS to ENABLE_TLS_VERIFY for odov2 test script
6882a2f 
Signed-off-by: Michael Valdron <[email protected]>
@michael-valdron
remove ENABLE_TLS_VERIFY for odov3 test script run under integration …
9fd8373 
…testing script

Signed-off-by: Michael Valdron <[email protected]>
@michael-valdron
revert showing full odo path in test log
8a2a9bb 
Signed-off-by: Michael Valdron <[email protected]>
@michael-valdron
openshift_integration: fix remaining file path references to use BASE…
b7aec49 
…_DIR instead of relative path

Signed-off-by: Michael Valdron <[email protected]>
@michael-valdron michael-valdron mentioned this pull request Jan 8, 2025
Open
3 tasks
@michael-valdron michael-valdron marked this pull request as ready for review January 8, 2025 20:55
@openshift-ci openshift-ci bot requested a review from johnmcollier January 8, 2025 20:55
@michael-valdron
Copy link
Member Author

/hold

Need to merge openshift/release#60441 before this PR can be merged.

@openshift-ci OpenShift CI
Copy link

openshift-ci bot commented Jan 8, 2025

@michael-valdron: 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
ci/prow/v4.15-registry-test b7aec49 link true /test v4.15-registry-test

Full PR test history. Your PR dashboard.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@elsony elsony Awaiting requested review from elsony

@Jdubrick Jdubrick Awaiting requested review from Jdubrick

@thepetk thepetk Awaiting requested review from thepetk

@johnmcollier johnmcollier Awaiting requested review from johnmcollier

Assignees

@thepetk thepetk

Labels
approved do-not-merge/hold
Projects
None yet
Milestone
No milestone
2 participants
@michael-valdron @thepetk