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

Fix PR integration testing #531

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

Conversation

michael-valdron
Copy link
Member

@michael-valdron michael-valdron commented Dec 24, 2024

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.

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
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

@@ -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
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

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

/hold

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

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
Projects
None yet
2 participants