Skip to content

[release-4.15] CORS-3169: Add Ingress LB IPs to Infra CR and set DNS unmanaged when BYO DNS is enabled#1022

Merged
openshift-merge-bot[bot] merged 4 commits intoopenshift:release-4.15from
sadasu:custom-dns-4.15
Feb 2, 2024
Merged

[release-4.15] CORS-3169: Add Ingress LB IPs to Infra CR and set DNS unmanaged when BYO DNS is enabled#1022
openshift-merge-bot[bot] merged 4 commits intoopenshift:release-4.15from
sadasu:custom-dns-4.15

Conversation

@sadasu
Copy link
Contributor

@sadasu sadasu commented Jan 31, 2024

Manual backport of #1016

  • Bring in the latest updates to Infrastructure CR from openshift/api
  • Update GCPPlatformStatus with the Ingress LB IPs when ClusterHosted DNS is enabled
  • Put DNS in Unmanaged state because an in-cluster DNS solution would be in use in place of the platform default DNS.

Based on enhancement: openshift/enhancements#1468

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jan 31, 2024
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Jan 31, 2024 •
edited by openshift-ci bot
Loading

@sadasu: This pull request references CORS-3169 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.15.0" version, but no target version was set.

Details

In response to this:

Manual backport of #1016

  • Bring in the latest updates to Infrastructure CR from openshift/api
  • Update GCPPlatformStatus with the Ingress LB IPs when ClusterHosted DNS is enabled
  • Put DNS in Unmanaged state because an in-cluster DNS solution would be in use in place of the platform default DNS.

Based on enhancement: openshift/enhancements#1468

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 openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci openshift-ci bot requested review from frobware and knobunc January 31, 2024 18:32
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 31, 2024

[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 candita for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details 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

@sadasu sadasu force-pushed the custom-dns-4.15 branch 2 times, most recently from 04c1f0e to e37c4f6 Compare January 31, 2024 21:43
@sadasu
Copy link
Contributor Author

sadasu commented Feb 1, 2024

/test e2e-azure-ovn

@sadasu
Copy link
Contributor Author

sadasu commented Feb 1, 2024

@Miciah, @gcs278 Could you PTAL?

gcs278
gcs278 reviewed Feb 1, 2024
View reviewed changes
go.mod Outdated
github.com/openshift/api => github.com/openshift/api v0.0.0-20231113114413-39964e6af314
k8s.io/client-go => k8s.io/client-go v0.28.0
github.com/openshift/api => github.com/openshift/api v0.0.0-20240126160819-e54a015ffc8c
k8s.io/client-go => k8s.io/client-go v0.29.0
Copy link
Contributor

@gcs278 gcs278 Feb 1, 2024

Choose a reason for hiding this comment

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

I don't think we need to or should rebase the K8S api changes on a backport. release-4.15 should be 0.28 packages.

Copy link
Contributor Author

@sadasu sadasu Feb 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Moving k8s.io/client-go's version back to v0.28.0 were resulting in these verify failures. https://prow.ci.openshift.org/view/gs/test-platform-results/pr-logs/pull/openshift_cluster-ingress-operator/1022/pull-ci-openshift-cluster-ingress-operator-release-4.15-verify/1753142958116311040

So moving it back to v0.29.0

Copy link
Contributor

@gcs278 gcs278 Feb 1, 2024

Choose a reason for hiding this comment

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

Did you roll back k8s.io/api v0.29.0 and k8s.io/apimachinery v0.29.0 as well? That passed verify for me locally. Sorry I meant all K8S packages.

I recommend to start with a clean go.mod from the latest commit in release-4.15 without your changes, then update github.com/openshift/api => github.com/openshift/api v0.0.0-20240126160819-e54a015ffc8c only and do go mod tidy and go mod vendor.

You'll get a much cleaner go.mod, I only see one other dependency bump (golang.org/x/exp). We should try to only bump with is specifically required for limiting the impact of this backport.

Copy link
Contributor Author

@sadasu sadasu Feb 2, 2024

Choose a reason for hiding this comment

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

Updated with minimal changes to go.mod.

@sadasu
Copy link
Contributor Author

sadasu commented Feb 1, 2024

/retest

1 similar comment
@sadasu
Copy link
Contributor Author

sadasu commented Feb 1, 2024

/retest

@sadasu sadasu force-pushed the custom-dns-4.15 branch 2 times, most recently from ceb7af5 to 398d619 Compare February 1, 2024 20:20
sadasu added 4 commits February 1, 2024 19:04
@sadasu
Updating openshift/api to bring in changes for BYO DNS for GCP
13a81e9 
Update vendoring as a result of this version bump
@sadasu
Update GCPPlatformStatus with Ingress LB IPs when BYO DNS is enabled
25b11d7
@sadasu
Set DNSManagementPolicy to Unmanaged when BYO DNS is enabled
6ff9dfe
@sadasu
Add/Update unit tests
712833e 
- Update test to verify the EndpointPublishingStrategy for GCP
- Add test to verify update of Infra CR with Ingress LB IPs
@sadasu
Copy link
Contributor Author

sadasu commented Feb 2, 2024

/retest-required

@sadasu
Copy link
Contributor Author

sadasu commented Feb 2, 2024

/retest

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Feb 2, 2024

@sadasu: all tests passed!

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

@sdodson
Copy link
Member

sdodson commented Feb 2, 2024

/label approved
I'm approving this backport while the feature is still behind TechPreviewNoUpgrade featuregate. We will require QE to finish their validation and CI testing to confirm that this new feature meets or exceeds 90% pass rate before moving it to on by default. This is not an assertion that this feature will be on by default in 4.15 GA or any time thereafter, that remains to be decided.

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 2, 2024
@sdodson
Copy link
Member

sdodson commented Feb 2, 2024

/label jira/valid-bug

@openshift-ci openshift-ci bot added the jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. label Feb 2, 2024
@gcs278
Copy link
Contributor

gcs278 commented Feb 2, 2024

No concern from me.
/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 2, 2024
@sadasu
Copy link
Contributor Author

sadasu commented Feb 2, 2024

@Miciah would you be able to add the backport-risk-assessed label?

@sdodson sdodson added cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. backport-risk-assessed Indicates a PR to a release branch has been evaluated and considered safe to accept. labels Feb 2, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit 0106cb9 into openshift:release-4.15 Feb 2, 2024
@openshift-bot
Copy link
Contributor

openshift-bot commented Feb 2, 2024

[ART PR BUILD NOTIFIER]

This PR has been included in build ose-cluster-ingress-operator-container-v4.15.0-202402022138.p0.g0106cb9.assembly.stream for distgit ose-cluster-ingress-operator.
All builds following this will include this PR.

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

Reviewers

@frobware frobware Awaiting requested review from frobware

@knobunc knobunc Awaiting requested review from knobunc

1 more reviewer

@gcs278 gcs278 gcs278 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@gcs278 gcs278

@lihongan lihongan

@melvinjoseph86 melvinjoseph86

@ShudiLi ShudiLi

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. backport-risk-assessed Indicates a PR to a release branch has been evaluated and considered safe to accept. cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@sadasu @openshift-ci-robot @sdodson @gcs278 @openshift-bot @lihongan @melvinjoseph86 @ShudiLi