Skip to content

Conversation

@Miciah
Copy link
Contributor

@Miciah Miciah commented Dec 11, 2025

Revert "OCPBUGS-62987: Move DynamicConfigurationManager back to DevPreview"

This reverts #2552.

@openshift-ci-robot
Copy link

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

@openshift-ci-robot openshift-ci-robot added jira/severity-important Referenced Jira bug's severity is important for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. labels Dec 11, 2025
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Dec 11, 2025

Hello @Miciah! Some important instructions when contributing to openshift/api:
API design plays an important part in the user experience of OpenShift and as such API PRs are subject to a high level of scrutiny to ensure they follow our best practices. If you haven't already done so, please review the OpenShift API Conventions and ensure that your proposed changes are compliant. Following these conventions will help expedite the api review process for your PR.

@openshift-ci-robot
Copy link

@Miciah: This pull request references Jira Issue OCPBUGS-67232, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (4.21.0) matches configured target version for branch (4.21.0)
  • bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @lihongan

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

Details

In response to this:

Revert "OCPBUGS-62987: Move DynamicConfigurationManager back to DevPreview"

This reverts #2552.

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 a review from lihongan December 11, 2025 17:33
@coderabbitai
Copy link

coderabbitai bot commented Dec 11, 2025

Walkthrough

IngressControllerDynamicConfigurationManager is enabled for TechPreviewNoUpgrade in addition to DevPreviewNoUpgrade; corresponding updates were made to the feature declaration, feature list doc, and two TechPreviewNoUpgrade FeatureGate manifests.

Changes

Cohort / File(s) Summary
Feature doc
features.md
Reordered table row: IngressControllerDynamicConfigurationManager re-added to show Enabled for TechPreviewNoUpgrade (Hypershift and SelfManagedHA); DevPreviewNoUpgrade remains Enabled.
Feature declaration (Go)
features/features.go
FeatureGateIngressControllerDynamicConfigurationManager: changed enhancementPR to "https://github.com/openshift/enhancements/pull/1687" and enableIn to include configv1.TechPreviewNoUpgrade alongside configv1.DevPreviewNoUpgrade.
TechPreviewNoUpgrade FeatureGate manifests
payload-manifests/featuregates/featureGate-Hypershift-TechPreviewNoUpgrade.yaml, payload-manifests/featuregates/featureGate-SelfManagedHA-TechPreviewNoUpgrade.yaml
IngressControllerDynamicConfigurationManager moved from status.disabled to status.enabled.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between 67eebfa and c2174d0.

📒 Files selected for processing (2)
  • features.md
  • features/features.go
🚧 Files skipped from review as they are similar to previous changes (2)
  • features/features.go
  • features.md

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.5.0)

Error: build linters: unable to load custom analyzer "kubeapilinter": tools/_output/bin/kube-api-linter.so, plugin: not implemented
The command is terminated due to an error: build linters: unable to load custom analyzer "kubeapilinter": tools/_output/bin/kube-api-linter.so, plugin: not implemented


Comment @coderabbitai help to get the list of available commands and usage tips.

@openshift-ci openshift-ci bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Dec 11, 2025
@Miciah
Copy link
Contributor Author

Miciah commented Dec 11, 2025

Cc'ing author and reviewers from the reverted PR:
/cc @candita
/cc @alebedev87
/cc @JoelSpeed

@openshift-ci openshift-ci bot requested review from alebedev87 and candita December 11, 2025 19:40
@JoelSpeed
Copy link
Contributor

/lgtm

BTW, this has no EP linked, is there any EP?

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 11, 2025
@openshift-ci-robot
Copy link

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-aws-ovn
/test e2e-aws-ovn-hypershift
/test e2e-aws-ovn-hypershift-conformance
/test e2e-aws-ovn-techpreview
/test e2e-aws-serial-1of2
/test e2e-aws-serial-2of2
/test e2e-aws-serial-techpreview-1of2
/test e2e-aws-serial-techpreview-2of2
/test e2e-azure
/test e2e-gcp
/test e2e-upgrade
/test e2e-upgrade-out-of-change

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 11, 2025
@Miciah
Copy link
Contributor Author

Miciah commented Dec 11, 2025

BTW, this has no EP linked, is there any EP?

Yes: https://github.com/openshift/enhancements/blob/master/enhancements/ingress/dynamic-config-manager.md

Should I update the PR with a reference?

@Miciah
Copy link
Contributor Author

Miciah commented Dec 11, 2025

Well, features/features.go links to PRs; openshift/enhancements#1687 is the PR for the EP.

@JoelSpeed
Copy link
Contributor

Can you update l633 to remove the legacy feature gate thing and add the actual Ep link in?

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Dec 11, 2025
@Miciah Miciah force-pushed the OCPBUGS-67232-move-DynamicConfigurationManager-back-to-TPNU branch from 053d7dc to 845ec5e Compare December 11, 2025 21:15
@Miciah
Copy link
Contributor Author

Miciah commented Dec 11, 2025

ci/prow/verify failed:

panic: FeatureGate/IngressControllerDynamicConfigurationManager enhancementPR format is incorrect; must be like https://github.com/openshift/enhancements/pull/#### or https://github.com/kubernetes/enhancements/issues/#### 

https://github.com/openshift/api/compare/053d7dc9eb425a835f21f151558ea7f2eb26b064..845ec5ef5e49ef176ee0567fa3d6c5f3d35ecb38 updates the link to point to the PR instead.

@JoelSpeed
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 12, 2025
@openshift-ci-robot
Copy link

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-aws-ovn
/test e2e-aws-ovn-hypershift
/test e2e-aws-ovn-hypershift-conformance
/test e2e-aws-ovn-techpreview
/test e2e-aws-serial-1of2
/test e2e-aws-serial-2of2
/test e2e-aws-serial-techpreview-1of2
/test e2e-aws-serial-techpreview-2of2
/test e2e-azure
/test e2e-gcp
/test e2e-upgrade
/test e2e-upgrade-out-of-change

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Dec 12, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JoelSpeed

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-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 16, 2025
Revert "OCPBUGS-62987: Move DynamicConfigurationManager back to DevPreview"

This reverts commit 25db151.

This commit resolves OCPBUGS-67232.

https://issues.redhat.com/browse/OCPBUGS-67232
@Miciah Miciah force-pushed the OCPBUGS-67232-move-DynamicConfigurationManager-back-to-TPNU branch from 845ec5e to 67eebfa Compare December 23, 2025 20:30
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Dec 23, 2025
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Dec 23, 2025

New changes are detected. LGTM label has been removed.

@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 23, 2025
@Miciah
Copy link
Contributor Author

Miciah commented Dec 23, 2025

* features/features.go
(FeatureGateIngressControllerDynamicConfigurationManager): Add a link
to the dynamic-config-manager enhancement proposal PR.
@Miciah Miciah force-pushed the OCPBUGS-67232-move-DynamicConfigurationManager-back-to-TPNU branch from 67eebfa to c2174d0 Compare December 24, 2025 03:03
@ShudiLi
Copy link
Member

ShudiLi commented Dec 24, 2025

Tested it with 4.21.0-0-2025-12-24-011621-test-ci-ln-ftxy4xk-latest

1.
% oc get clusterversion
NAME      VERSION                                                AVAILABLE   PROGRESSING   SINCE   STATUS
version   4.21.0-0-2025-12-24-011621-test-ci-ln-ftxy4xk-latest   True        False         64m     Cluster version is 4.21.0-0-2025-12-24-011621-test-ci-ln-ftxy4xk-latest

2.
% oc get featuregates cluster -oyaml | yq ".status.featureGates[0].disabled" | grep IngressControllerDynamicConfigurationManager
- name: IngressControllerDynamicConfigurationManager

3. enable the featuregate
 % oc patch featuregates cluster -p '{"spec": {"featureSet": "TechPreviewNoUpgrade"}}' --type=merge
featuregate.config.openshift.io/cluster patched

4.
% oc get featuregates cluster -oyaml | yq ".status.featureGates[0].enabled" | grep IngressControllerDynamicConfigurationManager
- name: IngressControllerDynamicConfigurationManager

% oc -n openshift-ingress get deployment router-default -oyaml | grep -i -A1 -E "ROUTER_MAX_DYNAMIC_SERVERS|ROUTER_BLUEPRINT_ROUTE_POOL_SIZE"
        - name: ROUTER_BLUEPRINT_ROUTE_POOL_SIZE
          value: "0"
--
        - name: ROUTER_MAX_DYNAMIC_SERVERS
          value: "1"

5.
 % oc get route
NAME          HOST/PORT                                                             PATH   SERVICES      PORT          TERMINATION   WILDCARD
unsec-apach   unsec-apach-default.apps.ci-ln-ftxy4xk-72292.gcp-2.ci.openshift.org          unsec-apach   unsec-apach                 None
% curl http://unsec-apach-default.apps.ci-ln-ftxy4xk-72292.gcp-2.ci.openshift.org 
It is a test!

6.
sh-5.1$ cat haproxy.config | grep -A26 "backend be_http:default:unsec-apach"
backend be_http:default:unsec-apach
  mode http
  option redispatch
  option forwardfor
  balance random

  timeout check 5000ms
  http-request add-header X-Forwarded-Host %[req.hdr(host)]
  http-request add-header X-Forwarded-Port %[dst_port]
  http-request add-header X-Forwarded-Proto http if !{ ssl_fc }
  http-request add-header X-Forwarded-Proto https if { ssl_fc }
  http-request add-header X-Forwarded-Proto-Version h2 if { ssl_fc_alpn -i h2 }
  http-request add-header Forwarded for=%[src];host=%[req.hdr(host)];proto=%[req.hdr(X-Forwarded-Proto)]
  cookie b94bb237dc742029fe83e6d395082b86 insert indirect nocache httponly dynamic
  server pod:appach-server-66b4878747-7cwl4:unsec-apach:unsec-apach:10.128.2.9:8080 10.128.2.9:8080 cookie 599df0d39ab4f2d33ebf5ff4f27652c6 weight 1 check inter 5000ms
  server pod:appach-server-66b4878747-xl5xn:unsec-apach:unsec-apach:10.128.2.10:8080 10.128.2.10:8080 cookie 8f24621644852474567e9d01c31849af weight 1 check inter 5000ms
  server pod:appach-server-66b4878747-pfswt:unsec-apach:unsec-apach:10.128.2.11:8080 10.128.2.11:8080 cookie 142a0857b6ac5260aac892cda144bfda weight 1 check inter 5000ms
  server pod:appach-server-66b4878747-xsfbh:unsec-apach:unsec-apach:10.128.2.12:8080 10.128.2.12:8080 cookie 9e385c2f34ba5bcb2ad11512e4413623 weight 1 check inter 5000ms
  server pod:appach-server-66b4878747-hjcnj:unsec-apach:unsec-apach:10.129.2.23:8080 10.129.2.23:8080 cookie 4795f79aa20da8f28cc164af2a702ff6 weight 1 check inter 5000ms
  server pod:appach-server-66b4878747-vzczb:unsec-apach:unsec-apach:10.129.2.24:8080 10.129.2.24:8080 cookie b9802b4c1977bdb382366a732be1fc65 weight 1 check inter 5000ms
  server pod:appach-server-66b4878747-vpvbr:unsec-apach:unsec-apach:10.129.2.25:8080 10.129.2.25:8080 cookie 33459e7024d45ce2c82d01ad68467cb2 weight 1 check inter 5000ms
  server pod:appach-server-66b4878747-57r4p:unsec-apach:unsec-apach:10.131.0.23:8080 10.131.0.23:8080 cookie fbfbf053ce7dde8d30c340926631e5f8 weight 1 check inter 5000ms
  server pod:appach-server-66b4878747-5jzw5:unsec-apach:unsec-apach:10.131.0.24:8080 10.131.0.24:8080 cookie 663c2f65d8deb8c910546bf9db9ccbe4 weight 1 check inter 5000ms
  server pod:appach-server-66b4878747-8rq66:unsec-apach:unsec-apach:10.131.0.25:8080 10.131.0.25:8080 cookie b5c2648c7a09d408e05aff2f210c5b86 weight 1 check inter 5000ms
  dynamic-cookie-key b94bb237dc742029fe83e6d395082b86
  server-template _dynamic-pod- 1-1 172.4.0.4:8765 check disabled

@ShudiLi
Copy link
Member

ShudiLi commented Dec 24, 2025

/label qe-approved
/verified by @ShudiLi

@openshift-ci openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Dec 24, 2025
@openshift-ci-robot
Copy link

@Miciah: This pull request references Jira Issue OCPBUGS-67232, which is valid.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (4.22.0) matches configured target version for branch (4.22.0)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @ShudiLi

Details

In response to this:

Revert "OCPBUGS-62987: Move DynamicConfigurationManager back to DevPreview"

This reverts #2552.

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 a review from ShudiLi December 24, 2025 03:22
@openshift-ci-robot openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Dec 24, 2025
@openshift-ci-robot
Copy link

@ShudiLi: This PR has been marked as verified by @ShudiLi.

Details

In response to this:

/label qe-approved
/verified by @ShudiLi

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.

@ShudiLi
Copy link
Member

ShudiLi commented Dec 24, 2025

/retest-required

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Dec 24, 2025

@Miciah: 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-serial-techpreview-1of2 845ec5e link true /test e2e-aws-serial-techpreview-1of2

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

@JoelSpeed
Copy link
Contributor

/override ci/prow/okd-scos-images

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Dec 24, 2025

@JoelSpeed: Overrode contexts on behalf of JoelSpeed: ci/prow/okd-scos-images

Details

In response to this:

/override ci/prow/okd-scos-images

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-sigs/prow repository.

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. 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. qe-approved Signifies that QE has signed off on this PR size/S Denotes a PR that changes 10-29 lines, ignoring generated files. verified Signifies that the PR passed pre-merge verification criteria

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants