OCPBUGS-70354: add serviceAccount to global-pull-secret-syncer by ehearne-redhat · Pull Request #7439 · openshift/hypershift

ehearne-redhat · 2026-01-08T13:49:10Z

What this PR does / why we need it:

Adds a bespoke service account to global-pull-secret-syncer.
We should not use default service account for our components.

Which issue(s) this PR fixes:

Fixes https://issues.redhat.com/browse/OCPBUGS-70354 .

Special notes for your reviewer:

Checklist:

Subject and description added to both, commit and PR.
Relevant issues have been referenced.
This change includes docs.
This change includes unit tests.

coderabbitai · 2026-01-08T13:49:16Z

Important

Review skipped

Auto reviews are limited based on label configuration.

🚫 Review skipped — only excluded labels are configured. (1)

do-not-merge/work-in-progress

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

Walkthrough

Reconcile now creates/updates a ServiceAccount for the GlobalPullSecret before syncing the DaemonSet; the DaemonSet PodSpec sets the ServiceAccountName. An e2e test change allows partial node presence for the OpenshiftOVNKubeDaemonSet readiness check.

Changes

Cohort / File(s)	Summary
ServiceAccount Manifest Definition `control-plane-operator/hostedclusterconfigoperator/controllers/resources/manifests/pullsecret.go`	Added `GlobalPullSecretServiceAccount()` returning a `corev1.ServiceAccount` with `Name` = `GlobalPullSecretDSName` and `Namespace` = `GlobalPullSecretNamespace`.
ServiceAccount Reconciliation & DaemonSet `control-plane-operator/hostedclusterconfigoperator/controllers/globalps/globalps.go`	`reconcileGlobalPullSecret()` now CreateOrUpdate(s) the ServiceAccount from `GlobalPullSecretServiceAccount()` at start; the reconciled DaemonSet PodSpec sets `ServiceAccountName` to `GlobalPullSecretDSName`.
E2E Test Readiness Adjustment `test/e2e/util/globalps.go`	Changed readiness verifier to allow partial node presence for `OpenshiftOVNKubeDaemonSet` (`AllowPartialNodes: true`).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

✨ Finishing touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

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

openshift-ci-robot · 2026-01-08T13:49:16Z

@ehearne-redhat: This pull request references Jira Issue OCPBUGS-70354, which is invalid:

expected the bug to target the "4.22.0" version, but no target version was set

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:

What this PR does / why we need it:

Adds a bespoke service account to global-pull-secret-syncer.

We should not use default service account for our components.

Which issue(s) this PR fixes:

Fixes https://issues.redhat.com/browse/OCPBUGS-70354 .

Special notes for your reviewer:

Checklist:

Subject and description added to both, commit and PR.

Relevant issues have been referenced.

This change includes docs.

This change includes unit tests.

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.

ehearne-redhat · 2026-01-08T13:50:01Z

/jira refresh

openshift-ci-robot · 2026-01-08T13:50:09Z

@ehearne-redhat: This pull request references Jira Issue OCPBUGS-70354, 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.22.0) matches configured target version for branch (4.22.0)
bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, POST)

No GitHub users were found matching the public email listed for the QA contact in Jira (yli2@redhat.com), skipping review request.

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 openshift-eng/jira-lifecycle-plugin repository.

ehearne-redhat · 2026-01-09T12:18:01Z

/retest

ehearne-redhat · 2026-01-09T14:29:45Z

/retest

ehearne-redhat · 2026-01-13T16:24:00Z

/retest

ehearne-redhat · 2026-02-11T15:46:28Z

/test verify-deps
/test e2e-v2-aws
/test e2e-kubevirt-aws-ovn-reduced
/test e2e-aws-upgrade-hypershift-operator
/test e2e-aws-4-21
/test e2e-aws
/test e2e-aks-4-21
/test e2e-aks

ehearne-redhat · 2026-02-11T15:48:51Z

/test all

ehearne-redhat · 2026-02-12T15:03:54Z

/retest

ehearne-redhat · 2026-02-13T09:09:36Z

/test e2e-aks

ehearne-redhat · 2026-02-26T11:53:46Z

/test e2e-aks
/test e2e-aws
/test e2e-aws-upgrade-hypershift-operator
/test e2e-kubevirt-aws-ovn-reduced
/test e2e-v2-aws

ehearne-redhat · 2026-02-26T15:16:09Z

/retest

ehearne-redhat · 2026-03-03T13:20:23Z

/test e2e-aws

ehearne-redhat · 2026-03-04T10:15:21Z

/retest

ehearne-redhat · 2026-03-04T10:16:02Z

/test e2e-aws

ehearne-redhat · 2026-03-04T16:57:33Z

/test e2e-aws

This fix adds a service account to global-pull-secret-syncer pod so it does not use default service account.

Add comprehensive unit test coverage for reconcileGlobalPullSecret() function with test cases covering: - Secret creation and merging when additional pull secret exists - Error handling for missing secrets and invalid JSON - InPlace NodePool handling - DaemonSet and ServiceAccount creation and configuration Set AllowPartialNodes to true for OVN daemonset in E2E validation to fix test failures when checking for ready daemonsets.

ehearne-redhat · 2026-03-05T12:12:34Z

/test e2e-aks
/test e2e-aws
/test e2e-aws-upgrade-hypershift-operator
/test e2e-kubevirt-aws-ovn-reduced
/test e2e-v2-aws

ehearne-redhat · 2026-03-05T14:59:19Z

/test e2e-v2-aws

bryan-cox

/approve

openshift-ci · 2026-03-09T16:36:15Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bryan-cox, ehearne-redhat

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~~ [bryan-cox]

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

ehearne-redhat · 2026-03-10T09:22:15Z

global-pull-secret-syncer pod is running with service account in e2e-v2-aws

---
apiVersion: v1
kind: Pod
metadata:
  ...
  name: global-pull-secret-syncer-n84zk
  namespace: kube-system
  ...
  serviceAccount: global-pull-secret-syncer
  serviceAccountName: global-pull-secret-syncer
  ...
    ...
  phase: Running
...

/verified by ehearne-redhat

openshift-ci-robot · 2026-03-10T09:22:26Z

@ehearne-redhat: This PR has been marked as verified by ehearne-redhat.

Details

In response to this:

global-pull-secret-syncer pod is running with service account in e2e-v2-aws

---
apiVersion: v1
kind: Pod
metadata:
 ...
 name: global-pull-secret-syncer-n84zk
 namespace: kube-system
 ...
 serviceAccount: global-pull-secret-syncer
 serviceAccountName: global-pull-secret-syncer
 ...
   ...
 phase: Running
...

/verified by ehearne-redhat

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.

everettraven

Aside from a minor comment on test structure, this LGTM.

If the HyperShift folks are happy with this PR, ship it :).

everettraven · 2026-03-10T14:21:51Z

+		expectGlobalSecretExists   bool
+		expectOriginalSecretExists bool
+		expectDaemonSetExists      bool
+		expectServiceAccountExists bool
+		expectError                bool
+		validateDaemonSet          func(*testing.T, *appsv1.DaemonSet)
+		validateGlobalSecret       func(*testing.T, *corev1.Secret)
+		validateOriginalSecret     func(*testing.T, *corev1.Secret)
+		validateServiceAccount     func(*testing.T, *corev1.ServiceAccount)


I generally find that using the validate* pattern makes table-driven tests much harder to read because each test may end up with slightly different ways to validate something which is more logic overhead to have to follow when trying to understand if there are any testing gaps.

I prefer taking a more declarative approach where expect* and validate* fields here (aside from maybe expectError) becomes something like:

expectedDaemonSet *appsv1.DaemonSet // and so on ...

everettraven · 2026-03-11T20:08:03Z

Not blocking this PR on test changes and HCP folks seem happy with the changes.

Code changes LGTM.

/lgtm

openshift-ci-robot · 2026-03-11T20:09:56Z

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-aks
/test e2e-aws
/test e2e-aws-upgrade-hypershift-operator
/test e2e-kubevirt-aws-ovn-reduced
/test e2e-v2-aws

ehearne-redhat · 2026-03-12T08:46:37Z

/retest

openshift-ci-robot · 2026-03-13T16:28:14Z

/retest-required

Remaining retests: 0 against base HEAD 259cead and 2 for PR HEAD 95dcbb0 in total

openshift-ci-robot · 2026-03-13T20:26:51Z

/retest-required

Remaining retests: 0 against base HEAD 2172048 and 1 for PR HEAD 95dcbb0 in total

openshift-ci · 2026-03-14T00:09:37Z

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

ehearne-redhat · 2026-03-16T11:54:14Z

/retest

openshift-ci-robot · 2026-03-16T14:23:38Z

@ehearne-redhat: Jira Issue Verification Checks: Jira Issue OCPBUGS-70354
✔️ This pull request was pre-merge verified.
✔️ All associated pull requests have merged.
✔️ All associated, merged pull requests were pre-merge verified.

Jira Issue OCPBUGS-70354 has been moved to the MODIFIED state and will move to the VERIFIED state when the change is available in an accepted nightly payload. 🕓

Details

In response to this:

What this PR does / why we need it:

Adds a bespoke service account to global-pull-secret-syncer.

We should not use default service account for our components.

Which issue(s) this PR fixes:

Fixes https://issues.redhat.com/browse/OCPBUGS-70354 .

Special notes for your reviewer:

Checklist:

Subject and description added to both, commit and PR.

Relevant issues have been referenced.

This change includes docs.

This change includes unit tests.

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 do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. do-not-merge/needs-area labels Jan 8, 2026

openshift-ci Bot requested review from enxebre and sjenning January 8, 2026 13:50

openshift-ci-robot added jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. and removed jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Jan 8, 2026

openshift-ci Bot added area/control-plane-operator Indicates the PR includes changes for the control plane operator - in an OCP release and removed do-not-merge/needs-area labels Jan 8, 2026

ehearne-redhat force-pushed the add-sa-global-pull-secret-syncer branch from 72691f1 to f18dcd4 Compare January 9, 2026 09:12

ehearne-redhat changed the title ~~[WIP] OCPBUGS-70354: add serviceAccount to global-pull-secret-syncer~~ OCPBUGS-70354: add serviceAccount to global-pull-secret-syncer Jan 9, 2026

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 Jan 9, 2026

ehearne-redhat force-pushed the add-sa-global-pull-secret-syncer branch from f18dcd4 to d769440 Compare January 12, 2026 12:54

openshift-ci Bot added the area/testing Indicates the PR includes changes for e2e testing label Jan 13, 2026

ehearne-redhat force-pushed the add-sa-global-pull-secret-syncer branch from c4cee48 to a661ff1 Compare January 13, 2026 12:17

gaol reviewed Feb 11, 2026

View reviewed changes

Comment thread control-plane-operator/hostedclusterconfigoperator/controllers/globalps/globalps.go

ehearne-redhat force-pushed the add-sa-global-pull-secret-syncer branch from a661ff1 to 684e4b1 Compare February 11, 2026 14:02

ehearne-redhat force-pushed the add-sa-global-pull-secret-syncer branch from 5da5668 to 7ac24b9 Compare February 26, 2026 11:16

ehearne-redhat added 2 commits March 5, 2026 10:39

fix(globalps): add serviceAccount to global-pull-secret-syncer

3f31f0d

This fix adds a service account to global-pull-secret-syncer pod so it does not use default service account.

ehearne-redhat force-pushed the add-sa-global-pull-secret-syncer branch from 7ac24b9 to 95dcbb0 Compare March 5, 2026 10:39

bryan-cox reviewed Mar 9, 2026

View reviewed changes

openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 9, 2026

openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Mar 10, 2026

everettraven reviewed Mar 10, 2026

View reviewed changes

openshift-ci Bot assigned everettraven Mar 11, 2026

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Mar 11, 2026

openshift-merge-bot Bot merged commit 112c720 into openshift:main Mar 16, 2026
23 checks passed

Conversation

ehearne-redhat commented Jan 8, 2026

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Checklist:

Uh oh!

coderabbitai Bot commented Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Walkthrough

Changes

Estimated code review effort

Uh oh!

openshift-ci-robot commented Jan 8, 2026

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Checklist:

Uh oh!

ehearne-redhat commented Jan 8, 2026

Uh oh!

openshift-ci-robot commented Jan 8, 2026

Uh oh!

ehearne-redhat commented Jan 9, 2026

Uh oh!

ehearne-redhat commented Jan 9, 2026

Uh oh!

ehearne-redhat commented Jan 13, 2026

Uh oh!

Uh oh!

ehearne-redhat commented Feb 11, 2026

Uh oh!

ehearne-redhat commented Feb 11, 2026

Uh oh!

ehearne-redhat commented Feb 12, 2026

Uh oh!

ehearne-redhat commented Feb 13, 2026

Uh oh!

ehearne-redhat commented Feb 26, 2026

Uh oh!

ehearne-redhat commented Feb 26, 2026

Uh oh!

ehearne-redhat commented Mar 3, 2026

Uh oh!

ehearne-redhat commented Mar 4, 2026

Uh oh!

ehearne-redhat commented Mar 4, 2026

Uh oh!

ehearne-redhat commented Mar 4, 2026

Uh oh!

ehearne-redhat commented Mar 5, 2026

Uh oh!

ehearne-redhat commented Mar 5, 2026

Uh oh!

bryan-cox left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci Bot commented Mar 9, 2026

Uh oh!

ehearne-redhat commented Mar 10, 2026

Uh oh!

openshift-ci-robot commented Mar 10, 2026

Uh oh!

everettraven left a comment

Choose a reason for hiding this comment

Uh oh!

everettraven Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

everettraven commented Mar 11, 2026

Uh oh!

openshift-ci-robot commented Mar 11, 2026

Uh oh!

ehearne-redhat commented Mar 12, 2026

Uh oh!

openshift-ci-robot commented Mar 13, 2026

Uh oh!

openshift-ci-robot commented Mar 13, 2026

Uh oh!

coderabbitai Bot commented Jan 8, 2026 •

edited

Loading