Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Package retries can result in Packages getting stuck in Pending #492

Closed
mjnagel opened this issue Jun 17, 2024 · 0 comments · Fixed by #511
Closed

Package retries can result in Packages getting stuck in Pending #492

mjnagel opened this issue Jun 17, 2024 · 0 comments · Fixed by #511
Assignees
Labels
bug Something isn't working operator Issues pertaining to the UDS Operator (Pepr)
Milestone

Comments

@mjnagel
Copy link
Contributor

mjnagel commented Jun 17, 2024

Steps to reproduce

  1. Create a package with a known good state.
  2. Wait for it to go to Ready.
  3. Modify the package to a known bad state (ex: use a uri in the sso client id)
  4. Observe that the package gets stuck in Pending without doing additional retries.

Expected result

Package should retry up to 5 times and then either get marked as Ready or Failed.

Actual Result

Package is stuck in Pending until a restart of the watcher pod.

Severity/Priority

Medium - this is very confusing to end users on what is happening.

Additional Context

Our shouldSkip logic filters out pending packages. During a retry we leave the status in Pending, resulting in it getting stuck unless it hits other cases (max retries or first time the watcher has seen it).

@mjnagel mjnagel added the possible-bug Something may not be working label Jun 17, 2024
@mjnagel mjnagel self-assigned this Jun 17, 2024
@mjnagel mjnagel added bug Something isn't working operator Issues pertaining to the UDS Operator (Pepr) and removed possible-bug Something may not be working labels Jul 2, 2024
@mjnagel mjnagel closed this as completed in cae5aab Jul 3, 2024
@mjnagel mjnagel added this to the 0.23.0 milestone Jul 3, 2024
rjferguson21 pushed a commit that referenced this issue Jul 11, 2024
## Description

This PR has a number of changes, mostly tied to operator behaviors and
bug fixes around failure logging. Included:
- URI Encoding of client ID on deletion/updates - when we call
updates/deletions on KC clients it gets appended to our URL for our
request and must be encoded
- Moves around the try/catch to only wrap the Keycloak API call so that
errors are surfaced more accurately in events and modifies the thrown
error from the Keycloak response to include the keycloak response status
and data (see #448)
- Adds a new Phase to Package for `Retrying` - this differentiates from
a `Pending` package that is already being reconciled to allow retries to
function as expected (see
#492)
- Moves uidSeen addition to patch status to handle infinite failure ->
pending -> failure ... loop on first apply (see
#525)

## Related Issue

Fixes #492

Fixes #448

Fixes #525

## Type of change

- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Other (security config, docs update, etc)

## Checklist before merging

- [x] Test, docs, adr added or updated as needed
- [x] [Contributor Guide
Steps](https://github.com/defenseunicorns/uds-template-capability/blob/main/CONTRIBUTING.md)(https://github.com/defenseunicorns/uds-template-capability/blob/main/CONTRIBUTING.md#submitting-a-pull-request)
followed

---------

Co-authored-by: Megamind <882485+jeff-mccoy@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working operator Issues pertaining to the UDS Operator (Pepr)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant