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

Use oc v4.11 instead v4.7 to fix version comparison #6559

Merged

Conversation

feloy
Copy link
Contributor

@feloy feloy commented Feb 1, 2023

What type of PR is this:

/kind tests

What does this PR do / why we need it:

Which issue(s) this PR fixes:

oc versions v4.7 and v4.11 use different information from the cluster to retrieve the OpenShift cluster version.

In odo, we send platform information to telemetry, and get information from the cluster the same way oc v4.11 is getting it.

In the tests, we currently compare the output of oc version v4.7 with what odo get, to check that we are sending the good version.

Unfortunalety, in some circumstances, oc v4.7 and v4.11 return different version.

This makes our tests fail in these circumstances.

Updating oc to version v4.11 for the tests should fix the problem.

Failing test: https://s3.eu-de.cloud-object-storage.appdomain.cloud/odo-tests-openshift-logs/pr-6545-openshift-tests-3993.html

This happens when cluster is updating:

$ kubectl get clusterversions.config.openshift.io version -o yaml
[...]
status:
  availableUpdates: null
  conditions:
  - lastTransitionTime: "2022-10-20T13:59:32Z"
    message: The update channel has not been configured.
    reason: NoChannel
    status: "False"
    type: RetrievedUpdates
  - lastTransitionTime: "2022-10-20T14:53:17Z"
    message: Done applying 4.9.52
    status: "True"
    type: Available
  - lastTransitionTime: "2022-11-16T12:24:52Z"
    status: "False"
    type: Failing
  - lastTransitionTime: "2023-02-01T12:53:09Z"
    message: 'Working towards 4.9.54: 447 of 561 done (79% complete), waiting on image-registry'
    reason: ClusterOperatorUpdating
    status: "True"
    type: Progressing
  desired:
    image: registry.eu-de.bluemix.net/armada-master/ocp-release:4.9.54-x86_64
    url: https://access.redhat.com/errata/RHSA-2022:9111
    version: 4.9.54
  history:
  - completionTime: null
    image: registry.eu-de.bluemix.net/armada-master/ocp-release:4.9.54-x86_64
    startedTime: "2023-02-01T12:53:09Z"
    state: Partial
    verified: false
    version: 4.9.54
  - completionTime: "2022-12-16T01:31:28Z"
    image: registry.eu-de.bluemix.net/armada-master/ocp-release:4.9.52-x86_64
    startedTime: "2022-12-16T01:19:43Z"
    state: Completed
    verified: false
    version: 4.9.52

PR acceptance criteria:

  • Unit test

  • Integration test

  • Documentation

How to test changes / Special notes to the reviewer:

@feloy feloy requested a review from anandrkskd February 1, 2023 14:29
@feloy feloy requested a review from rm3l February 1, 2023 14:29
@netlify
Copy link

netlify bot commented Feb 1, 2023

Deploy Preview for odo-docusaurus-preview canceled.

Name Link
🔨 Latest commit 5cee589
🔍 Latest deploy log https://app.netlify.com/sites/odo-docusaurus-preview/deploys/63da7753af9c12000826e3a0

@openshift-ci openshift-ci bot requested a review from kadel February 1, 2023 14:30
@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 1, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@odo-robot
Copy link

odo-robot bot commented Feb 1, 2023

OpenShift Unauthenticated Tests on commit finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Feb 1, 2023

NoCluster Tests on commit finished successfully.
View logs: TXT HTML

@feloy feloy changed the title Use oc v4.11 instead v4.7 to fix version retrieval Use oc v4.11 instead v4.7 to fix version comparison Feb 1, 2023
@odo-robot
Copy link

odo-robot bot commented Feb 1, 2023

Unit Tests on commit finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Feb 1, 2023

Validate Tests on commit finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Feb 1, 2023

Windows Tests (OCP) on commit finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Feb 1, 2023

Kubernetes Tests on commit finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Feb 1, 2023

OpenShift Tests on commit finished with errors.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Feb 1, 2023

Kubernetes Docs Tests on commit e0ab28c finished successfully.
View logs: TXT HTML

@feloy
Copy link
Contributor Author

feloy commented Feb 2, 2023

/override OpenShift-Integration-tests/OpenShift-Integration-tests
Image will be updated when PR merged, test is still failing

@openshift-ci
Copy link

openshift-ci bot commented Feb 2, 2023

@feloy: Overrode contexts on behalf of feloy: OpenShift-Integration-tests/OpenShift-Integration-tests

In response to this:

/override OpenShift-Integration-tests/OpenShift-Integration-tests
Image will be updated when PR merged, test is still failing

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/test-infra repository.

Copy link
Member

@rm3l rm3l left a comment

Choose a reason for hiding this comment

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

oc versions v4.7 and v4.11 use different information from the cluster to retrieve the OpenShift cluster version.

In odo, we send platform information to telemetry, and get information from the cluster the same way oc v4.11 is getting it.

In the tests, we currently compare the output of oc version v4.7 with what odo get, to check that we are sending the good version.

Unfortunalety, in some circumstances, oc v4.7 and v4.11 return different version.

This makes our tests fail in these circumstances.

Updating oc to version v4.11 for the tests should fix the problem.

LGTM, but I am now wondering if our tests could not be more resilient to avoid similar issues in the future.. Maybe we could just check that the platform version we send as telemetry property only matches a certain regular expression, no? What do you think?

@anandrkskd
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Feb 2, 2023
@feloy
Copy link
Contributor Author

feloy commented Feb 2, 2023

oc versions v4.7 and v4.11 use different information from the cluster to retrieve the OpenShift cluster version.
In odo, we send platform information to telemetry, and get information from the cluster the same way oc v4.11 is getting it.
In the tests, we currently compare the output of oc version v4.7 with what odo get, to check that we are sending the good version.
Unfortunalety, in some circumstances, oc v4.7 and v4.11 return different version.
This makes our tests fail in these circumstances.
Updating oc to version v4.11 for the tests should fix the problem.

LGTM, but I am now wondering if our tests could not be more resilient to avoid similar issues in the future.. Maybe we could just check that the platform version we send as telemetry property only matches a certain regular expression, no? What do you think?

This way, the test also checks the detection of the version is aligned with oc version (provided that we have the correct oc binary)

@feloy
Copy link
Contributor Author

feloy commented Feb 2, 2023

/override ci/prow/v4.11-integration-e2e
Not related

@openshift-ci
Copy link

openshift-ci bot commented Feb 2, 2023

@feloy: Overrode contexts on behalf of feloy: ci/prow/v4.11-integration-e2e

In response to this:

/override ci/prow/v4.11-integration-e2e
Not related

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/test-infra repository.

@openshift-merge-robot openshift-merge-robot merged commit 4b2e935 into redhat-developer:main Feb 2, 2023
@rm3l rm3l added this to the v3.7.0 🚀 milestone Feb 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm Indicates that a PR is ready to be merged. Required by Prow.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants