OCPBUGS-74528: upkeep: HighlyAvailableArbiter has been GA for 2 releases#2759
OCPBUGS-74528: upkeep: HighlyAvailableArbiter has been GA for 2 releases#2759eggfoobar wants to merge 1 commit intoopenshift:masterfrom
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
jira/valid-reference
|
Hello @eggfoobar! Some important instructions when contributing to openshift/api: |
|
@eggfoobar: This pull request references Jira Issue OCPBUGS-74528, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
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
bot
added
the
size/XXL
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (39)
📒 Files selected for processing (29)
💤 Files with no reviewable changes (17)
✅ Files skipped from review due to trivial changes (10)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR removes the exported HighlyAvailableArbiter feature gate from registration and deletes its entries from multiple FeatureGate payload manifests and the features.md table. Several CRD test files and generated feature-gated CRD manifest entries referencing HighlyAvailableArbiter were removed or updated. Validation annotations for InfrastructureStatus.controlPlaneTopology were simplified to a unified FeatureGateAwareEnum mapping. Multiple CRD and ControllerConfig schema descriptions were extended to document a HighlyAvailableArbiter mode (2 full control-plane nodes + 1 smaller arbiter). A special-case match for arbiter in the featuregate test analyzer was also removed. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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.11.3)Error: build linters: unable to load custom analyzer "kubeapilinter": tools/_output/bin/kube-api-linter.so, plugin: not implemented Comment |
ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan Review Summary by QodoRemove HighlyAvailableArbiter feature gate - GA for 2 releases
WalkthroughsDescription• Remove HighlyAvailableArbiter feature gate after GA stabilization • Make HighlyAvailableArbiter topology mode always available • Update validation rules to allow arbiter mode unconditionally • Fix code formatting and indentation in feature gate definitions Diagramflowchart LR
A["HighlyAvailableArbiter<br/>Feature Gate"] -->|Remove| B["Always Available<br/>Topology Mode"]
C["Feature Gate<br/>Validation Rules"] -->|Simplify| D["Unconditional<br/>Enum Values"]
E["CRD Manifests"] -->|Update| F["Include Arbiter<br/>in All Profiles"]
File Changes1. features/features.go
|
Code Review by Qodo
1. controlPlaneTopology enum undocumented
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
|
Looks like the integration tests still reference the gate, that will need to be removed What payload E2E can we run to check that this PR doesn't break any existing jobs? Is there a way to run an arbiter job? |
eggfoobar
force-pushed
the
upkeep-remove-arbiter-featuregate
branch
from
405f89c to
821a1ab
Compare
Ah yup, updated the integration tests! As it stands the installer should be the only one that checks for the feature gate, the other tooling relies on the control plane topology or compute count to execute flows. We should be able to execute some payloads on this PR, but I think the cluster config operator update will be the one to show any failures. I'll open a PR there with a temp update to go mod to validate |
|
/payload-job periodic-ci-openshift-release-main-nightly-4.22-e2e-metal-ovn-two-node-arbiter |
|
@eggfoobar: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/42c8af30-2237-11f1-9a8b-2e91671672d5-0 |
| featureGates: | ||
| - DualReplica | ||
| - HighlyAvailableArbiter |
There was a problem hiding this comment.
Are these tests no longer relevant? We can keep them if they still make sense, just gotta drop the gates
There was a problem hiding this comment.
Oh this was that sort of hack we had to do because DualReplica and HighlyAvailableArbiter were occupying the same field, this file was just a merging of the tests in DualReplica and HighlyAvailableArbiter files, now that HighlyAvailableArbiter is default, we don't need this file anymore.
The tests this was checking for are still present with their respective DualReplica and HighlyAvailableArbiter file.
.../controllerconfigs.machineconfiguration.openshift.io/HighlyAvailableArbiter+DualReplica.yaml
Show resolved
Hide resolved
|
/lgtm |
|
@JoelSpeed: This PR has been marked as verified by DetailsIn response to this:
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-robot
added
the
verified
|
Scheduling tests matching the |
openshift-ci
bot
added
the
approved
|
/retest-required |
1 similar comment
|
/retest-required |
|
/retest-required Running one more retest, errors do not seem to be related. Will check tomorrow if this fails again. |
removing the HighlyAvailableArbiter featureGate, the feature has been GA for 2 releases updated doc to mention what HighlyAvailableArbiter does Signed-off-by: ehila <ehila@redhat.com>
eggfoobar
force-pushed
the
upkeep-remove-arbiter-featuregate
branch
from
821a1ab to
73d0cb6
Compare
openshift-ci-robot
removed
the
verified
|
Had forgotten to update the docs comment |
|
/verified by CI Only doc update in the force push |
openshift-ci-robot
added
the
verified
|
@eggfoobar: This PR has been marked as verified by DetailsIn response to this:
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. |
|
/retest-required |
|
/lgtm |
|
Pipeline controller notification The |
|
[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 DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
3 participants
removing the HighlyAvailableArbiter featureGate, the feature has been GA for 2 releases