Skip to content

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

Merged
openshift-merge-robot merged 1 commit intoopenshift:release-4.12from
gcs278:release-4.12-protocolStrategy
Sep 8, 2023
Merged

[release-4.12] OCPBUGS-15251: Add support for protocolStrategy API field to enable force_tcp configuration#378
openshift-merge-robot merged 1 commit intoopenshift:release-4.12from
gcs278:release-4.12-protocolStrategy

Conversation

@gcs278
Copy link
Copy Markdown
Contributor

@gcs278 gcs278 commented Aug 7, 2023

This is a manual cherry-pick of #359 because the API (openshift/api#1500) is also being backported as openshift/api#1502 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 openshift-ci-robot added the jira/severity-important Referenced Jira bug's severity is important for the branch this PR is targeting. label Aug 7, 2023
@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 7, 2023
@openshift-ci-robot openshift-ci-robot added jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Aug 7, 2023
@openshift-ci-robot
Copy link
Copy Markdown
Contributor

@gcs278: This pull request references Jira Issue OCPBUGS-6829, which is invalid:

  • expected the bug to be in one of the following states: NEW, ASSIGNED, POST, but it is Verified instead

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

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.

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 7, 2023
@openshift-ci openshift-ci Bot requested review from Miciah and candita August 7, 2023 18:11
@openshift-ci-robot
Copy link
Copy Markdown
Contributor

@gcs278: This pull request references Jira Issue OCPBUGS-6829, which is invalid:

  • expected the bug to be in one of the following states: NEW, ASSIGNED, POST, but it is Verified instead

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

Details

In response to this:

This is a manual cherry-pick of #359 because the API (openshift/api#1500) is also being backported as openshift/api#1502 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 gcs278 changed the base branch from master to release-4.12 August 7, 2023 18:11
@openshift-ci-robot
Copy link
Copy Markdown
Contributor

@gcs278: This pull request references Jira Issue OCPBUGS-6829, which is invalid:

  • expected the bug to target the "4.12.z" version, but it targets "4.14.0" instead
  • expected the bug to be in one of the following states: NEW, ASSIGNED, POST, but it is Verified instead
  • expected Jira Issue OCPBUGS-6829 to depend on a bug targeting a version in 4.13.0, 4.13.z and in one of the following states: VERIFIED, RELEASE PENDING, CLOSED (ERRATA), CLOSED (CURRENT RELEASE), CLOSED (DONE), but no dependents were found

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

Details

In response to this:

This is a manual cherry-pick of #359 because the API (openshift/api#1500) is also being backported as openshift/api#1502 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 gcs278 changed the title [WIP] [release-4.12] OCPBUGS-6829: Add support for protocolStrategy API field to enable force_tcp configuration [WIP] [release-4.12] OCPBUGS-15251: Add support for protocolStrategy API field to enable force_tcp configuration Aug 7, 2023
@openshift-ci-robot
Copy link
Copy Markdown
Contributor

@gcs278: This pull request references Jira Issue OCPBUGS-15251, which is invalid:

  • expected dependent Jira Issue OCPBUGS-16720 to be in one of the following states: VERIFIED, RELEASE PENDING, CLOSED (ERRATA), CLOSED (CURRENT RELEASE), CLOSED (DONE), but it is ON_QA instead
  • expected dependent Jira Issue OCPBUGS-16720 to target a version in 4.13.0, 4.13.z, but it targets "4.14.0" instead

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

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 as openshift/api#1502 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 gcs278 force-pushed the release-4.12-protocolStrategy branch from 80ea0c5 to 8114249 Compare August 7, 2023 18:15
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 7, 2023
@melvinjoseph86
Copy link
Copy Markdown

/retest-required

@melvinjoseph86
Copy link
Copy Markdown

/retest-required

…orce_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 gcs278 force-pushed the release-4.12-protocolStrategy branch from 8114249 to 5f89cdd Compare August 16, 2023 15:00
@gcs278 gcs278 changed the title [WIP] [release-4.12] OCPBUGS-15251: Add support for protocolStrategy API field to enable force_tcp configuration [release-4.12] OCPBUGS-15251: Add support for protocolStrategy API field to enable force_tcp configuration Aug 16, 2023
@openshift-ci openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 16, 2023
@gcs278
Copy link
Copy Markdown
Contributor Author

gcs278 commented Aug 16, 2023

/jira refresh

@openshift-ci-robot
Copy link
Copy Markdown
Contributor

@gcs278: This pull request references Jira Issue OCPBUGS-15251, which is invalid:

  • expected dependent Jira Issue OCPBUGS-16720 to be in one of the following states: VERIFIED, RELEASE PENDING, CLOSED (ERRATA), CLOSED (CURRENT RELEASE), CLOSED (DONE), but it is ON_QA instead
  • expected dependent Jira Issue OCPBUGS-16720 to target a version in 4.13.0, 4.13.z, but it targets "4.14.0" instead

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

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

Details

In response to this:

/jira refresh

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.

@candita
Copy link
Copy Markdown
Contributor

candita commented Aug 16, 2023

/assign @Miciah

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Aug 16, 2023

@gcs278: 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.

@gcs278
Copy link
Copy Markdown
Contributor Author

gcs278 commented Aug 24, 2023

/jira refresh

@openshift-ci-robot
Copy link
Copy Markdown
Contributor

@gcs278: This pull request references Jira Issue OCPBUGS-15251, which is invalid:

  • expected dependent Jira Issue OCPBUGS-16720 to be in one of the following states: VERIFIED, RELEASE PENDING, CLOSED (ERRATA), CLOSED (CURRENT RELEASE), CLOSED (DONE), but it is ON_QA instead
  • expected dependent Jira Issue OCPBUGS-16720 to target a version in 4.13.0, 4.13.z, but it targets "4.14.0" instead

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

Details

In response to this:

/jira refresh

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
Copy link
Copy Markdown
Contributor Author

gcs278 commented Aug 24, 2023

/jira refresh

@openshift-ci-robot openshift-ci-robot added the jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. label Aug 24, 2023
@openshift-ci-robot
Copy link
Copy Markdown
Contributor

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

6 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (4.12.z) matches configured target version for branch (4.12.z)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)
  • dependent bug Jira Issue OCPBUGS-15225 is in the state Closed (Done), which is one of the valid states (VERIFIED, RELEASE PENDING, CLOSED (ERRATA), CLOSED (CURRENT RELEASE), CLOSED (DONE))
  • dependent Jira Issue OCPBUGS-15225 targets the "4.13.z" version, which is one of the valid target versions: 4.13.0, 4.13.z
  • bug has dependents

Requesting review from QA contact:
/cc @melvinjoseph86

Details

In response to this:

/jira refresh

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-robot openshift-ci-robot removed the jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. label Aug 24, 2023
@openshift-ci openshift-ci Bot requested a review from melvinjoseph86 August 24, 2023 18:25
@candita
Copy link
Copy Markdown
Contributor

candita commented Sep 6, 2023

/assign

@candita
Copy link
Copy Markdown
Contributor

candita commented Sep 6, 2023

Pulling in @frobware or @knobunc. I'm not sure how to review the backport of vendored openshift/api go modules for this bug fix. The vendored openshift/api code being backported seems like much more than what is needed by this bug fix. Is that anything to worry about?

@Miciah
Copy link
Copy Markdown
Contributor

Miciah commented Sep 7, 2023

/approve
/lgtm

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

The one difference that catches my attention vis-à-vis the release-4.13 backport PR, #372, and this PR is that bumping openshift/api brings in a lot of reformatting of the descriptions in the CRD, but these changes are all cosmetic, and possibly improvements. I'm curious whether this PR improves the output of the numbered list in the oc explain DNS.spec.upstreamResolvers.transportConfig.tls.caBundle --api-version=operator.openshift.io/v1 output, for example. Anyway, the formatting changes should not affect functionality.
/label backport-risk-assessed

@openshift-ci openshift-ci Bot added the backport-risk-assessed Indicates a PR to a release branch has been evaluated and considered safe to accept. label Sep 7, 2023
@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Sep 7, 2023
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Sep 7, 2023

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

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

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 7, 2023
@Miciah
Copy link
Copy Markdown
Contributor

Miciah commented Sep 7, 2023

Pulling in @frobware or @knobunc. I'm not sure how to review the backport of vendored openshift/api go modules for this bug fix. The vendored openshift/api code being backported seems like much more than what is needed by this bug fix. Is that anything to worry about?

Repeating from internal discussion for transparency: The large number of changes result from the infrequent openshift/api bumps in cluster-dns-operator's release-4.12 branch, and most of the changes have no impact on cluster-dns-operator. For example, this operator does not use the IngressController API or instantiate its CRD, and so the changes to vendor/github.com/openshift/api/operator/v1/types_ingress.go that come with bumping openshift/api in cluster-dns-operator do not have any impact.

@melvinjoseph86
Copy link
Copy Markdown

/label cherry-pick-approved

@openshift-ci 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 Sep 8, 2023
@openshift-merge-robot openshift-merge-robot merged commit d149aee into openshift:release-4.12 Sep 8, 2023
@openshift-ci-robot
Copy link
Copy Markdown
Contributor

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

Jira Issue OCPBUGS-15251 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 as openshift/api#1502 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-merge-robot
Copy link
Copy Markdown
Contributor

Fix included in accepted release 4.12.0-0.nightly-2023-09-08-181253

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

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/severity-important Referenced Jira bug's severity is important for the branch this PR is targeting. 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

Development

Successfully merging this pull request may close these issues.

8 participants