OCPBUGS-86949: Guard HCCO KubeletConfig CM deletion against transient source absence - #8671
Closed
vsolanki12 wants to merge 79 commits into
Closed
OCPBUGS-86949: Guard HCCO KubeletConfig CM deletion against transient source absence#8671vsolanki12 wants to merge 79 commits into
vsolanki12 wants to merge 79 commits into
Conversation
Add onUpdate test cases for Services, ControllerAvailabilityPolicy, and Capabilities immutability to the existing envtest YAML test suites. CEL XValidation rules enforce these fields at the CRD level, but no onUpdate test cases previously existed to guard against accidental CEL marker removal. Ref: GCP-688 Assisted-by: Claude:claude-opus-4-6
…t webhooks Add unit tests covering three v1 validation equivalents: - EnsureCustomTolerations: verify toleration propagation from hcp.Spec.Tolerations to pod template in the controlplane-component framework (the analogous labels test existed; tolerations were missed) - ValidateConfigurationStatus: verify HCCO hcpstatus controller reads guest Authentication and writes to HCP status - EnsureGuestWebhooksValidated: verify ensureGuestAdmissionWebhooksAreValid matching/deletion logic with a fake client, including the URL allowlist for control plane service endpoints Ref: GCP-688 Assisted-by: Claude:claude-opus-4-6
Port the remaining v1 TestCreateCluster validations to the v2 test framework as Ginkgo specs, enabling retirement of the e2e-gke CI job. Framework extensions: - Add pod exec helpers (test/e2e/v2/util/pod_exec.go) for running commands in guest cluster pods - Add REST config and service domain resolution to test context - Add centralized test context validators (ValidateHostedCluster, ValidateGuestCluster) replacing per-test boilerplate New specs covering: - ValidateHostedClusterConditions: validate ~20 HC conditions - EnsureNoCrashingPods: check container RestartCount with tolerations - EnsureCAPIFinalizers: validate CAPI component finalizers - EnsureGuestWebhooksValidated: create and validate webhook deletion - EnsureAdmissionPolicies: validate 5 VAPs exist and enforce correctly - EnsureNetworkPolicies: validate pod labels and live curl enforcement - EnsureKubeAPIDNSNameCustomCert: custom TLS cert integration - EnsureMetricsForwarderWorking: multi-hop metrics pipeline validation - ValidateMetrics: operator metrics endpoint scraping - EnsureNodeTuningOperatorMetricsEndpoint: NTO service/metrics check - EnsureFeatureGateStatus: guest FeatureGate matches ClusterVersion - EnsureCustomLabels: sweep HCP pods for custom labels - EnsureCustomTolerations: sweep HCP pods for custom tolerations - EnsureAllRoutesUseHCPRouter: sweep routes for HCP router label Also applies centralized test context validators and BDD naming conventions to existing test files touched during porting. Ref: GCP-688 Assisted-by: Claude:claude-opus-4-6
Remove the Informing label from WorkloadRegistryValidationTest and InfrastructureRegistryValidationTest so that unrecognized pods in both the control plane and infrastructure namespaces fail CI instead of being silently skipped. Also applies centralized test context validator and cleans up redundant comments. Ref: GCP-688 Assisted-by: Claude:claude-opus-4-6
Move EnsureNoCrashingPodsTest from hosted_cluster_health_test.go to control_plane_workloads_test.go using the per-workload pattern with individual It blocks per registered workload. Remove hosted cluster mutation from EnsureMetricsForwarderWorkingTest so it follows the non-lifecycle verification pattern: skip if the metrics forwarding annotation is not already set instead of setting it. Assisted-by: Claude:claude-opus-4-6
…attern Move EnsureCustomLabelsTest and EnsureCustomTolerationsTest from hosted_cluster_compliance_test.go to control_plane_workloads_test.go using the per-workload pattern with individual It blocks per registered workload, matching the reviewer-requested convention. Assisted-by: Claude:claude-opus-4-6
- Wrap final Prometheus metrics query in Eventually to prevent CI flakes from scrape timing races - Port certificate rotation and leader election exception paths from v1 NoCrashingPods to prevent false test failures - Add error type discrimination to network policy egress test so it catches exec/binary failures rather than silently passing - Wrap OperatorHub update in Eventually to match the Network update retry pattern in the same test - Add private-router egress check to network policy test for parity with v1 coverage - Replace manual toleration loop with Gomega ContainElement matcher - Fix context.Background() usage in getWorkloadPods helper - Change t.Errorf to t.Fatalf in webhook assertion default branches - Extract expectMetricHasLabel helper to deduplicate metric label checks - Add ClusterVersion condition propagation tests covering CVO mapping, nil Upgradeable defaulting, and empty Reason fallback - Remove redundant what-comments in infrastructure test - Fix E2E_SERVICE_DOMAIN env var description accuracy Assisted-by: Claude:claude-opus-4-6
… helpers Use ValidateHostedCluster/ValidateHostedClusterClient helpers instead of inline GetHostedCluster + Expect(hc).NotTo(BeNil()) and GetHostedClusterClient + Expect(guestClient).NotTo(BeNil()) patterns across all four PR openshift#8527 lifecycle test files. Also convert bare defer cleanup blocks to DeferCleanup with apierrors.IsNotFound guards in nodepool_lifecycle_test.go to match the established cleanup pattern. Assisted-by: Claude:claude-opus-4-6
Capture patterns and anti-patterns identified during PR openshift#8511 review: - §13: Non-lifecycle tests must not mutate the hosted cluster - §14: Per-workload test placement in control_plane_workloads_test.go - §15: IPv6-safe URL construction via net.JoinHostPort - §16: Vacuous pass prevention by asserting non-empty lists - Gotcha: MicroShift guards don't apply to v2 e2e tests - Architecture: document util/, lifecycle/, cmd/ packages - Fix envtest version range from 1.31-1.35 to 1.30-1.35 Assisted-by: Claude:claude-opus-4-6
Disable Docstrings, MicroShift, SNO, and OTE pre-merge checks that are inherited from the org but do not apply to HyperShift. Assisted-by: Claude:claude-opus-4-6
- Move workload.Name from It() titles into Context() blocks across all per-workload test loops to eliminate dynamic test names - Split EnsureAdmissionPoliciesTest into 4 separate It blocks with Ordered + BeforeAll for shared setup - Add assertion context to bare Expect(err) in payload arch test - Update AGENTS.md section 14 example to match Context() pattern Assisted-by: Claude:claude-opus-4-6
Weekly dependabot dependency consolidation.
Vendor updates for root module dependency changes.
Additional generated file updates from dependency changes.
…amping Add unit and E2E tests for OCPBUGS-78152 / CNTRLPLANE-3040 to prevent CAS from accumulating replica decrements on paused MachineDeployments. Unit tests: 9 table-driven cases covering replica clamping for both MachineDeployment and MachineSet upgrade types, including non-AWS effectiveMin=1 enforcement. E2E test: verifies CAS respects pause annotation by polling CAS logs for paused node group detection, with deferred unpause cleanup. Extracts shared helpers and constants to reduce duplication across autoscaling tests. Refs: OCPBUGS-78152, CNTRLPLANE-3040 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com>
Bumps [codecov/codecov-action](https://github.com/codecov/codecov-action) from 6.0.0 to 6.0.1. - [Release notes](https://github.com/codecov/codecov-action/releases) - [Changelog](https://github.com/codecov/codecov-action/blob/main/CHANGELOG.md) - [Commits](codecov/codecov-action@57e3a13...e79a696 354) --- updated-dependencies: - dependency-name: codecov/codecov-action dependency-version: 6.0.1 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
The webhook server requires TLS certs on disk before it can start listening. Previously, certs were only created via `hypershift install` manifests, meaning the operator would fail to start if the serving cert secret was not yet present like it is the case when running `hyperhsift install render` without the `--render-sensitive` flag. This is used in a scenario where the rendered manifests are pushed to a gitops workflow and, therefore, secrets can not be rendered. By adding bootstrap cert generation at startup, the operator is self-sufficient: if the secret exists, the volume mount delivers certs normally; if it is missing or empty, certs are generated, persisted, and written to disk. The secret volume is now marked optional so the pod can start without it. Signed-off-by: Borja Clemente <bclement@redhat.com>
Test the new certs secret bootstrapping logic in the hypershift-operator. Signed-off-by: Borja Clemente <bclement@redhat.com>
Cert secret bootstrapping now lives in the hypershift_operator and the CLI no longer needs to create the secrets. Signed-off-by: Borja Clemente <bclement@redhat.com>
The openshift-oauth-apiserver was started with --tls-min-version but without --tls-cipher-suites, unlike standalone OCP and other CPO-managed components (KCM, kube-scheduler). This adds the cipher suites arg using config.CipherSuites(), following the same pattern as kube-controller-manager.
When ConnectDirectlyToCloudAPIs is enabled, the konnectivity sidecar's dialDirectWithProxy() reads HTTPS_PROXY from the process environment to route cloud API calls through the management cluster's outbound proxy. The sidecar container spec was missing these env vars, so the proxy lookup returned empty and direct connections failed on clusters that require an outbound proxy. Conditionally call proxy.SetEnvVars() when building the konnectivity sidecar container spec, scoped to containers where ConnectDirectlyToCloudAPIs is true (HTTPS or Socks5 mode). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ARC runner containers use dash as default shell and don't have gh CLI installed. Use bash explicitly for the Claude installer and curl for the GitHub API call. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ARC runner containers use dash as default shell and don't have gh CLI installed. Use bash explicitly for the Claude installer and curl for the GitHub API call. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ARC runner containers use dash as default shell and don't have gh CLI installed. Use bash explicitly for the Claude installer and curl for the GitHub API call. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The container: directive requires containerMode on ARC runners which needs anyuid SCC and a long-running process. Replace it by installing Claude and cloning ai-helpers directly in workflow steps. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace fuse-overlayfs with Go 1.24+'s GOCACHEPROG protocol for serving the EFS-backed Go build cache. fuse-overlayfs required user namespaces (hostUsers: false) which fails on EFS because NFS does not support idmapped mounts on RHEL 9's kernel 5.14. gocacheprog is a small Go program (~180 lines, stdlib only) that implements the GOCACHEPROG JSON-over-stdin/stdout protocol. GET requests read from the read-only EFS cache first, then fall back to a writable local directory. PUT requests write to the local directory only. This gives zero-copy cache reads with no special SCC, no user namespaces, and no FUSE — works with the default restricted-v2 SCC. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add sync.WaitGroup to drain in-flight goroutines before exit - Guard actionFile/outputFile against empty IDs to prevent panic - Check errors from os.MkdirAll in handlePut - Use req.BodySize instead of len(req.Body) for action entry size - Move mkdir inside gocacheprog availability check in action.yaml - Remove fuse-overlayfs from Dockerfile (no longer needed) - Add comprehensive unit tests with race detection coverage Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The contrib/ directory is excluded by .dockerignore but the runner Dockerfile needs contrib/ci/gocacheprog/ in its build context. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
jira/severity-low
|
@vsolanki12: This pull request references Jira Issue OCPBUGS-86949, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
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. |
Contributor
|
Important Review skippedToo many files! This PR contains 231 files, which is 81 over the limit of 150. To get a review, narrow the scope: ⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (63)
📒 Files selected for processing (231)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
openshift-ci
Bot
added
do-not-merge/needs-area
do-not-merge/work-in-progress
Contributor
|
Skipping CI for Draft Pull Request. |
openshift-ci
Bot
added
the
needs-rebase
Contributor
|
PR needs rebase. DetailsInstructions 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. |
|
@vsolanki12: This pull request references Jira Issue OCPBUGS-86949. The bug has been updated to no longer refer to the pull request using the external bug tracker. All external bug links have been closed. The bug has been moved to the NEW state. 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
area/ai
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: vsolanki12 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
openshift-ci
Bot
added
area/api
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
16 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
HCCO's
reconcileKubeletConfigdeletes guest-side ConfigMaps whose source is absent from the HCP namespace. During the immutable-to-mutable migration (OCPBUGS-85778) or any transient API error, the source CM can be briefly absent, causing HCCO to delete the guest copy. NTO then regenerates MachineConfigs without it, triggering an MCO node rollout.This PR:
NTOMirroredConfigLabel, since their source is expected to reappear on the next reconcile cycledeleteImmutableConfigMapIfNeeded: Refactors to useDeleteIfNeededWithPredicatewith aKubeletConfigConfigMapLabelownership check, preventing accidental deletion of unrelated immutable ConfigMapsResourceVersionandImmutableafterDeleteIfNeededWithPredicateto avoid stale-resourceVersion errors and immutable leakage on the subsequentCreateOrUpdateWhich issue(s) this PR fixes:
Fixes OCPBUGS-86949
Special notes for your reviewer:
Checklist: