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

Fixing id:72514,id:68939,id:63139,id:63152 and updating proxy image #865

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jerichokeyne
Copy link
Contributor

@jerichokeyne jerichokeyne commented Jan 7, 2025

What this PR does / why we need it: Fixes OCP-72514, OCP-68939, OCP-63139, and OCP-63152 tests. Also fixes installing a classic cluster with a proxy enabled

Which issue(s) this PR fixes (optional, use fixes #<issue_number>(, fixes #<issue_number>, ...) format, where issue_number might be a GitHub issue, or a Jira story (OCM-xxxx):
Fixes #OCM-13194

Change type

  • New feature
  • Bug fix
  • Build
  • CI
  • Documentation
  • Performance
  • Refactor
  • Style
  • Unit tests
  • Subsystem tests

Checklist

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.

@openshift-ci openshift-ci bot requested review from jameszwang and oriAdler January 7, 2025 16:40
Copy link

openshift-ci bot commented Jan 7, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign yasun1 for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found 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

@jerichokeyne
Copy link
Contributor Author

/retest

// clusterArgs.OpenshiftVersion = helper.StringPointer(targetV)
// _, err = clusterService.Apply(clusterArgs)
// Expect(err).To(HaveOccurred())
// Expect(err.Error()).To(ContainSubstring("Missing required acknowledgements to schedule upgrade"))
Copy link
Contributor

Choose a reason for hiding this comment

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

May I know why common above checkpoint of missing upgrade_acknowledge?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's currently a bug that causes the upgrade to occur even without acknowledging it. I left it commented out so that once that bug gets fixed, we can just re-enable the test. It's also already disabled for the HCP upgrade:

By("Create github idp with empty hostname")
args = getDefaultGitHubArgs(idpName)
args.HostedDomain = helper.EmptyStringPointer
validateIDPArgAgainstErrorSubstrings(idpServices.github, args, "Attribute 'hostname' is mandatory")
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any reason to remove this above step By("Create github idp with empty hostname")?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As far as I can tell creating an IDP without the hostname set is how you're supposed to create one that uses github.com, so it seems like it's not a mandatory argument anymore

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants