[release-4.13] OCPBUGS-15225: Add support for protocolStrategy API field to enable force_tcp configuration by gcs278 · Pull Request #372 · openshift/cluster-dns-operator

gcs278 · 2023-07-11T15:05:53Z

This is a manual cherry-pick of #359 because the API (openshift/api#1500) is also being backported and needed to be manually vendored.

go.mod: Vendor API support for protocolStrategy
pkg/operator/controller/controller_dns_configmap.go: Logic to add force_tcp in corefile if protocolStrategy is TCP
pkg/operator/controller/controller_dns_configmap_test.go: Unit tests pkg/operator/controller/testdata/*: New corefiles for unit testing

openshift-ci-robot · 2023-07-11T15:06:03Z

@gcs278: This pull request references Jira Issue OCPBUGS-15225, which is valid.

6 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.13.z) matches configured target version for branch (4.13.z)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)
dependent bug Jira Issue OCPBUGS-6829 is in the state Verified, which is one of the valid states (VERIFIED, RELEASE PENDING, CLOSED (ERRATA), CLOSED (CURRENT RELEASE), CLOSED (DONE))
dependent Jira Issue OCPBUGS-6829 targets the "4.14.0" version, which is one of the valid target versions: 4.14.0
bug has dependents

Requesting review from QA contact:
/cc @melvinjoseph86

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

This is a manual cherry-pick of #359 because the API (openshift/api#1500) is also being backported and needed to be manually vendored.

go.mod: Vendor API support for protocolStrategy
pkg/operator/controller/controller_dns_configmap.go: Logic to add force_tcp in corefile if protocolStrategy is TCP
pkg/operator/controller/controller_dns_configmap_test.go: Unit tests pkg/operator/controller/testdata/*: New corefiles for unit testing

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.

gcs278 · 2023-07-11T15:09:39Z

There shouldn't be a merge conflict, tide is confused with #371
/test tide

openshift-ci · 2023-07-11T15:09:43Z

@gcs278: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test e2e-aws-ovn
/test e2e-aws-ovn-operator
/test e2e-aws-ovn-serial
/test e2e-aws-ovn-upgrade
/test images
/test unit
/test verify

The following commands are available to trigger optional jobs:

/test e2e-aws-ovn-single-node

Use /test all to run all jobs.

Details

In response to this:

There shouldn't be a merge conflict, tide is confused with #371
/test tide

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.

Miciah

I have a question about which commit to bump to in go.mod, but this PR looks good other than that.

Miciah · 2023-07-11T23:04:38Z

 	github.com/google/go-cmp v0.5.9
 	github.com/kevinburke/go-bindata v3.11.0+incompatible
-	github.com/openshift/api v0.0.0-20230120195050-6ba31fa438f2
+	github.com/openshift/api v0.0.0-20230620212129-894c5465a5ac


Do you want openshift/api@894c546 or openshift/api@b5a47f9 (the merge commit from the release-4.13 branch)? For the original PR, you used openshift/api@333bc19 (the merge commit from the main branch). It might not matter as the history is linear, but it seems like it would be good to be consistent.

The bump openshift/api@6ba31fa...894c546 (or openshift/api@6ba31fa...b5a47f9) pulls in a lot of unrelated commits, but none that looks like it would affect the DNS operator or API.

Yea good catch. I usually do merge commits, I think I got confused because when I did the git cherry-pick for this PR I had to select the non-merge commit a9eff7f for cluster-dns-operator, but that's somewhat irrelevant, since we are talking about the API vendoring commit here.

Updated to use the merge commit.

…rce_tcp configuration `go.mod`: Vendor API support for protocolStrategy `pkg/operator/controller/controller_dns_configmap.go`: Logic to add force_tcp in corefile if protocolStrategy is TCP `pkg/operator/controller/controller_dns_configmap_test.go`: Unit tests `pkg/operator/controller/testdata/*`: New corefiles for unit testing

gcs278 · 2023-07-12T14:47:15Z

/hold
aos-art gave the OK for openshift/api#1500 to be merged into the next 4.13.z release while this PR is still pending.

However, once this PR merges, the backport feature will go live and I feel we should also have docs ready and merged for the same 4.13.z backport. The 4.14 bug https://issues.redhat.com/browse/OCPBUGS-15833 will need to be merged and cherry-picked to 4.13.z, so blocking this PR on https://issues.redhat.com/browse/OCPBUGS-15833.

CC: @jmanthei

candita · 2023-07-12T15:38:14Z

/assign @Miciah

Miciah · 2023-07-12T15:44:26Z

/approve
/lgtm
This is a low-risk change that adds and implements a new API field in the DNS config. This new field defaults to empty and has no effect unless the cluster-admin specifies a nonempty value for it.
/label backport-risk-assessed

openshift-ci · 2023-07-12T15:53:50Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Miciah

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [Miciah]

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

melvinjoseph86 · 2023-07-12T16:33:53Z

/label cherry-pick-approved

gcs278 · 2023-07-12T18:32:53Z

lots of issues with e2e-aws-ovn-single-node, looks like major failure
/test e2e-aws-ovn-single-node

disruption testing
/test e2e-aws-ovn-upgrade

openshift-ci · 2023-07-12T20:47:56Z

@gcs278: 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/e2e-aws-ovn-single-node	`b3eb7d0`	link	false	`/test e2e-aws-ovn-single-node`

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.

gcs278 · 2023-07-13T17:50:23Z

/unhold
Spoke with @jboxman-rh about how the product documentation gets updated, he is aware of this backport and will generate a update.

openshift-ci-robot · 2023-07-13T17:56:27Z

@gcs278: Jira Issue OCPBUGS-15225: All pull requests linked via external trackers have merged:

Jira Issue OCPBUGS-15225 has been moved to the MODIFIED state.

Details

In response to this:

This is a manual cherry-pick of #359 because the API (openshift/api#1500) is also being backported and needed to be manually vendored.

go.mod: Vendor API support for protocolStrategy
pkg/operator/controller/controller_dns_configmap.go: Logic to add force_tcp in corefile if protocolStrategy is TCP
pkg/operator/controller/controller_dns_configmap_test.go: Unit tests pkg/operator/controller/testdata/*: New corefiles for unit testing

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-ci Bot requested review from alebedev87, knobunc and melvinjoseph86 July 11, 2023 15:06

Miciah reviewed Jul 11, 2023

View reviewed changes

gcs278 force-pushed the force_tcp-4.13 branch from 1fd9ef8 to b3eb7d0 Compare July 12, 2023 14:27

openshift-ci Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 12, 2023

openshift-ci Bot assigned Miciah Jul 12, 2023

openshift-ci Bot added the backport-risk-assessed Indicates a PR to a release branch has been evaluated and considered safe to accept. label Jul 12, 2023

openshift-ci Bot assigned lihongan, melvinjoseph86 and ShudiLi Jul 12, 2023

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jul 12, 2023

openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 12, 2023

openshift-ci Bot added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Jul 12, 2023

openshift-ci Bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 13, 2023

openshift-merge-robot merged commit 3ea8e9e into openshift:release-4.13 Jul 13, 2023

Miciah mentioned this pull request Sep 7, 2023

[release-4.12] OCPBUGS-15251: Add support for protocolStrategy API field to enable force_tcp configuration #378

Merged

Conversation

gcs278 commented Jul 11, 2023

Uh oh!

openshift-ci-robot commented Jul 11, 2023

Uh oh!

gcs278 commented Jul 11, 2023

Uh oh!

openshift-ci Bot commented Jul 11, 2023

Uh oh!

Miciah left a comment

Choose a reason for hiding this comment

Uh oh!

Miciah Jul 11, 2023

Choose a reason for hiding this comment

Uh oh!

gcs278 Jul 12, 2023

Choose a reason for hiding this comment

Uh oh!

gcs278 commented Jul 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

candita commented Jul 12, 2023

Uh oh!

Miciah commented Jul 12, 2023

Uh oh!

openshift-ci Bot commented Jul 12, 2023

Uh oh!

melvinjoseph86 commented Jul 12, 2023

Uh oh!

gcs278 commented Jul 12, 2023

Uh oh!

openshift-ci Bot commented Jul 12, 2023

Uh oh!

gcs278 commented Jul 13, 2023

Uh oh!

openshift-ci-robot commented Jul 13, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

gcs278 commented Jul 12, 2023 •

edited

Loading