Skip to content

Tolerate secrets being missing (22.0) - #23741

Merged
vmuzikar merged 1 commit into
keycloak:release/22.0from
Jamstah:reconcile-fix
Oct 6, 2023
Merged

Tolerate secrets being missing (22.0)#23741
vmuzikar merged 1 commit into
keycloak:release/22.0from
Jamstah:reconcile-fix

Conversation

@Jamstah

@Jamstah Jamstah commented Oct 5, 2023

Copy link
Copy Markdown
Contributor

If any secret was missing, the reconcile would error out. After doing this a number of times, the controller would stop trying and end up in a state where the operator would need restarting, or the resource would need recreating, to continue reconciliation.

This fix stops the reconcile from erroring out so the operator will reconcile continuously until the secrets are all present.

The fix is different than the fix for 23.0 because 23.0 has changed how secrets are handled.

Fixes #22170

@Jamstah
Jamstah requested review from a team as code owners October 5, 2023 12:59
@ghost ghost added the team/cloud-native label Oct 5, 2023
@Jamstah Jamstah changed the title Tolerate secrets being missing Tolerate secrets being missing (22.0) Oct 5, 2023
@Jamstah

Jamstah commented Oct 5, 2023

Copy link
Copy Markdown
Contributor Author

Having written this, I thought I would need to know if there were missing secrets for the logic to work, but actually, I didn't.

Everything for updating the status with the missing secret names could also be removed from this PR if you want it to be really small.

@shawkins

shawkins commented Oct 5, 2023

Copy link
Copy Markdown
Contributor

Having written this, I thought I would need to know if there were missing secrets for the logic to work, but actually, I didn't.

The changes look good, they will work fine for not optional secrets. Do you care about the optional case?

Everything for updating the status with the missing secret names could also be removed from this PR if you want it to be really small.

Probably best to remove from this pr. Adding this would need to tracked separately as we don't have that on main either.

@Jamstah

Jamstah commented Oct 5, 2023

Copy link
Copy Markdown
Contributor Author

Done, now its tiny :)

If any secret was missing, the reconcile would error out. After doing
this a number of times, the controller would stop trying and end up in a
state where the operator would need restarting, or the resource would
need recreating, to continue reconciliation.

This fix stops the reconcile from erroring out so the operator will
reconcile continuously until the secrets are all present.

Fixes keycloak#22170

@shawkins shawkins left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@vmuzikar vmuzikar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I think we don’t need to consider the optional Secrets here to keep it simple. This is fixed in main.

@vmuzikar

vmuzikar commented Oct 6, 2023

Copy link
Copy Markdown
Contributor

Thank you @Jamstah! :)

@vmuzikar
vmuzikar enabled auto-merge (squash) October 6, 2023 07:35
@vmuzikar vmuzikar linked an issue Oct 6, 2023 that may be closed by this pull request
2 tasks
@vmuzikar
vmuzikar merged commit 8bc37cc into keycloak:release/22.0 Oct 6, 2023
@cypress

cypress Bot commented Oct 6, 2023

Copy link
Copy Markdown

Passing run #9263 ↗︎

0 527 48 0 Flakiness 0

Details:

Tolerate secrets being missing (#23741)
Project: Keycloak Admin UI Commit: 8bc37ccbb0
Status: Passed Duration: 17:55 💡
Started: Oct 6, 2023 8:58 AM Ended: Oct 6, 2023 9:16 AM

Review all test suite changes for PR #23741 ↗︎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Operator secrets sequencing

3 participants