OCPBUGS-45895: allow reconciling existing Secrets #2539

rexagod · 2024-12-17T10:22:49Z

Until now, CMO did not reconcile existing secrets, even if their data changed. This changes that behavior.

I added CHANGELOG entry for this change.
No user facing changes, so no entry in CHANGELOG was needed.

openshift-ci-robot · 2024-12-17T10:22:56Z

@rexagod: This pull request references Jira Issue OCPBUGS-45895, which is invalid:

expected the bug to target the "4.19.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.

In response to this:

Until now, CMO did not reconcile existing secrets, even if their data changed. This changes that behavior.

I added CHANGELOG entry for this change.

No user facing changes, so no entry in CHANGELOG was needed.

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.

pkg/tasks/alertmanager.go

rexagod · 2024-12-17T10:54:51Z

/jira refresh

openshift-ci-robot · 2024-12-17T10:54:58Z

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

Requesting review from QA contact:
/cc @juzhao

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.

rexagod · 2025-01-13T11:19:06Z

I'm currently having some problems with make versions generate on MacOS. I'll rebase for the time being.

rexagod · 2025-01-20T09:06:34Z

/retest-required

machine424

thanks for the test, some suggestions.

Documentation/deps-versions.md

test/e2e/reconcile_objects_test.go

juzhao · 2025-02-05T08:42:52Z

/label qe-approved

openshift-ci-robot · 2025-02-05T08:42:59Z

@rexagod: This pull request references Jira Issue OCPBUGS-45895, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.19.0) matches configured target version for branch (4.19.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @juzhao

In response to this:

Until now, CMO did not reconcile existing secrets, even if their data changed. This changes that behavior.

I added CHANGELOG entry for this change.

No user facing changes, so no entry in CHANGELOG was needed.

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 · 2025-03-18T07:58:54Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rexagod

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [rexagod]

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

rexagod · 2025-03-20T08:07:26Z

okd-scos-e2e-aws-ovn errors are unrelated, so this is good for reviews.

machine424

Thanks again, we're almost ready to merge this, just some final suggestions

machine424 · 2025-04-01T09:14:37Z

test/e2e/reconcile_objects_test.go

+`)
+	f.MustCreateOrUpdateConfigMap(t, userWorkloadConfigMap)
+	defer f.MustDeleteConfigMap(t, userWorkloadConfigMap)
+	for _, secret := range []types.NamespacedName{


maybe we can just check that len(syncedSecrets)>5 or something to be sure the check isn't no-op?

Used a require.NotEmpty(t, syncedSecrets) right after we fill syncedSecrets to ensure it's not empty.

then I don't see why this f.AssertSecretExists is needed.

test/e2e/reconcile_objects_test.go

machine424 · 2025-04-01T09:30:42Z

test/e2e/reconcile_objects_test.go

+	require.NotEmpty(t, syncedSecrets)
+
+	// Update the aforementioned secrets' data.
+	for _, secret := range append(syncedSecrets, unsyncedSecrets...) {


maybe we can just have secrets instead of syncedSecrets with all the secrets and only do the skipping below.

Not sure what this means, could you please elaborate a bit? There's no secrets IIUC but only syncedSecrets and unsyncedSecrets.

I meant we can only have secrets with all secrets and loop over them only once and then use the slices.Contains check to see whether we want to // Check if the secrets were reconciled as expected. or // Check if the secrets were reconciled unexpectedly.. so we only have one check loop.
(this is a nit, I just find it more readable that way)

test/e2e/reconcile_objects_test.go

Documentation/deps-versions.md

rexagod · 2025-04-16T09:37:50Z

🤦🏼 So I just noticed I had a patch on local for this that I never pushed, I did that now, and I think some of the review comments are addressed by it. I'll go over them again to make sure I act on them.

machine424 · 2025-04-16T11:51:59Z

test/e2e/reconcile_objects_test.go

+	"k8s.io/apimachinery/pkg/types"
+)
+
+func TestSecretsReconciliation(t *testing.T) {


let's add a short comment about the overall logic in the test

machine424 · 2025-04-16T11:57:25Z

test/e2e/reconcile_objects_test.go

+func TestSecretsReconciliation(t *testing.T) {
+	// Create assets under both scenarios for us to work with.
+	setupUserWorkloadAssetsWithTeardownHook(t, f)
+	userWorkloadConfigMap := f.BuildUserWorkloadConfigMap(t, `alertmanager:


good catch, I think this config should make CMO produce all/most of the secrets.

rexagod · 2025-04-17T08:04:03Z

Need to test this out on a cluster, will request a review once that's green.

rexagod · 2025-04-17T10:08:17Z

/retest

juzhao · 2025-04-22T03:08:18Z

/retest

machine424 · 2025-05-06T09:46:35Z

/retest-required

rexagod · 2025-05-06T16:40:17Z

This needs to be updated a bit, will request a review once I clean this up.

Until now, CMO did not reconcile existing secrets, even if their data changed. This changes that behavior. Signed-off-by: Pranshu Srivastava <[email protected]>

openshift-ci · 2025-09-16T12:41:47Z

@rexagod: The following tests 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/generate	`1846128`	link	true	`/test generate`
ci/prow/e2e-agnostic-operator	`1846128`	link	true	`/test e2e-agnostic-operator`
ci/prow/e2e-aws-ovn	`1846128`	link	true	`/test e2e-aws-ovn`
ci/prow/ginkgo-tests	`1846128`	link	true	`/test ginkgo-tests`

Full PR test history. Your PR dashboard.

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.

openshift-merge-robot · 2025-09-16T12:41:54Z

PR needs rebase.

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.

openshift-ci bot requested review from machine424 and slashpai December 17, 2024 10:23

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 17, 2024

machine424 reviewed Dec 17, 2024

View reviewed changes

pkg/tasks/alertmanager.go Outdated Show resolved Hide resolved

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 Dec 17, 2024

openshift-ci bot requested a review from juzhao December 17, 2024 10:55

rexagod force-pushed the OCPBUGS-45895 branch from d5d9b9f to fc606f7 Compare December 18, 2024 07:09

rexagod force-pushed the OCPBUGS-45895 branch from f6d2966 to 8ba4e67 Compare January 13, 2025 10:16

rexagod force-pushed the OCPBUGS-45895 branch from 8ba4e67 to a81fdf4 Compare January 13, 2025 11:19

machine424 reviewed Jan 20, 2025

View reviewed changes

Documentation/deps-versions.md Outdated Show resolved Hide resolved

test/e2e/reconcile_objects_test.go Outdated Show resolved Hide resolved

test/e2e/reconcile_objects_test.go Outdated Show resolved Hide resolved

openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Feb 5, 2025

rexagod requested a review from machine424 February 24, 2025 08:29

rexagod force-pushed the OCPBUGS-45895 branch from 60a0a77 to 7500adc Compare March 18, 2025 07:58

rexagod force-pushed the OCPBUGS-45895 branch 5 times, most recently from 34754e6 to b088579 Compare March 19, 2025 09:00

rexagod force-pushed the OCPBUGS-45895 branch from 69695fe to 2d93123 Compare March 19, 2025 22:06

machine424 reviewed Apr 1, 2025

View reviewed changes

rexagod force-pushed the OCPBUGS-45895 branch from 2d93123 to aa76436 Compare April 16, 2025 09:35

rexagod force-pushed the OCPBUGS-45895 branch 3 times, most recently from c2589f3 to b5e48bd Compare April 16, 2025 10:18

rexagod requested a review from machine424 April 16, 2025 10:23

machine424 reviewed Apr 16, 2025

View reviewed changes

rexagod force-pushed the OCPBUGS-45895 branch from b5e48bd to f9e695a Compare April 17, 2025 08:02

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 6, 2025

OCPBUGS-45895: allow reconciling existing Secrets

19928f2

Until now, CMO did not reconcile existing secrets, even if their data changed. This changes that behavior. Signed-off-by: Pranshu Srivastava <[email protected]>

rexagod force-pushed the OCPBUGS-45895 branch from ec66b67 to 06053ce Compare July 21, 2025 07:13

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 21, 2025

rexagod added 2 commits July 22, 2025 21:36

chore: add tests for secret updation behavior

d5b34d6

chore: make versions

1846128

rexagod force-pushed the OCPBUGS-45895 branch from 06053ce to 1846128 Compare July 22, 2025 16:24

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 16, 2025

OCPBUGS-45895: allow reconciling existing Secrets #2539

Are you sure you want to change the base?

OCPBUGS-45895: allow reconciling existing Secrets #2539

Conversation

rexagod commented Dec 17, 2024

Uh oh!

openshift-ci-robot commented Dec 17, 2024

Uh oh!

Uh oh!

rexagod commented Dec 17, 2024

Uh oh!

openshift-ci-robot commented Dec 17, 2024

Uh oh!

rexagod commented Jan 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rexagod commented Jan 20, 2025

Uh oh!

machine424 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

juzhao commented Feb 5, 2025

Uh oh!

openshift-ci-robot commented Feb 5, 2025

Uh oh!

openshift-ci bot commented Mar 18, 2025

Uh oh!

rexagod commented Mar 20, 2025

Uh oh!

machine424 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rexagod commented Apr 16, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rexagod commented Apr 17, 2025

Uh oh!

rexagod commented Apr 17, 2025

Uh oh!

juzhao commented Apr 22, 2025

Uh oh!

machine424 commented May 6, 2025

Uh oh!

rexagod commented May 6, 2025

Uh oh!

openshift-ci bot commented Sep 16, 2025

Uh oh!

openshift-merge-robot commented Sep 16, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

rexagod commented Jan 13, 2025 •

edited

Loading