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.

Sign up for GitHub

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

Jump to bottom

fix: operator retries and error logging #511

Merged
merged 23 commits into from
Jul 3, 2024
Merged

fix: operator retries and error logging #511

merged 23 commits into from
Jul 3, 2024

Conversation

mjnagel
Copy link
Contributor

@mjnagel mjnagel commented Jun 26, 2024 •
edited
Loading

Description

This PR has a number of changes, mostly tied to operator behaviors and bug fixes around failure logging. Included:

Related Issue

Fixes #492

Fixes #448

Fixes #525

Type of change

  • 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

@mjnagel mjnagel self-assigned this Jun 26, 2024
@mjnagel mjnagel added this to the 0.23.0 milestone Jun 28, 2024
@mjnagel mjnagel marked this pull request as ready for review June 28, 2024 21:40
@mjnagel mjnagel requested a review from a team as a code owner June 28, 2024 21:40
rjferguson21
rjferguson21 previously approved these changes Jun 28, 2024
View reviewed changes
UnicornChance
UnicornChance previously approved these changes Jun 28, 2024
View reviewed changes
@mjnagel mjnagel requested a review from jeff-mccoy June 28, 2024 22:44
@mjnagel mjnagel dismissed stale reviews from UnicornChance and rjferguson21 via e603eaa June 28, 2024 23:17
@mjnagel mjnagel marked this pull request as draft June 29, 2024 01:10
jeff-mccoy and others added 6 commits July 1, 2024 20:22
@jeff-mccoy @mjnagel
fix: operator retry bug fixes (#524)
d2960a9 
This PR fixes two bugs around operator reconciliation/retry handling:
- Infinite pending/failure loop caused by placement of uidSeen addition
- Package retry new status phase to prevent packages getting stuck in
pending

Fixes #
<!-- or -->
Relates to #

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

- [ ] Test, docs, adr added or updated as needed
- [ ] [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: Micah Nagel <micah.nagel@defenseunicorns.com>
@mjnagel
fix: extra uidseen
ffaf345
@mjnagel
chore: simplify changes
faf61aa
@mjnagel
wip: add eventing around retry
805efce
@mjnagel
fix: format, pepr version
026219d
@mjnagel
Merge branch 'main' into operator-retry-logging
53edf77
@mjnagel mjnagel mentioned this pull request Jul 2, 2024
Closed
@mjnagel mjnagel marked this pull request as ready for review July 3, 2024 18:11
@UnicornChance
Copy link
Contributor

Something of note to capture in a future PR, creating and using a UDS Error type that allows for some better error handling and management.

  • tested deleting admin ui clients and verified they are recreated by reconciling package
  • tested deleting pepr store toke and new retry / error handling was expected broken state
  • tested deleting both token and admin ui clients and reconcolied package and client was successfully recreated
  • tested a few different package related edge cases like deleting and redeploying a package and verified the creation of a new client or in the case of an existing client a broken state
  • tested creating a client with invalid syntax to test that putting that token in the pepr store broke things as well

@mjnagel mjnagel merged commit cae5aab into main Jul 3, 2024
11 checks passed
@mjnagel mjnagel deleted the operator-retry-logging branch July 3, 2024 19:57
@github-actions github-actions bot mentioned this pull request Jul 3, 2024
Merged
mjnagel pushed a commit that referenced this pull request Jul 8, 2024
@github-actions
chore(main): release 0.23.0 (#482)
1513768 
🤖 I have created a release *beep* *boop*
---


##
[0.23.0](v0.22.2...v0.23.0)
(2024-07-04)


### ⚠ BREAKING CHANGES

* remove emulated gitlab endpoints from keycloak
([#483](#483))

### Features

* identity group auth
([#497](#497))
([d71d83e](d71d83e))


### Bug Fixes

* **docs:** re-ordered small paragraphs, clarified wording, and added
links to tech homepages
([#531](#531))
([6b2b46b](6b2b46b))
* **docs:** removed double-link which broke the markdown formatting in
pr template
([#532](#532))
([f41ced4](f41ced4))
* **docs:** uds-config.yaml example in k3d-slim-dev README
([#530](#530))
([2e1c53e](2e1c53e))
* operator retries and error logging
([#511](#511))
([cae5aab](cae5aab))


### Miscellaneous

* **deps:** update checkout action to latest sha
([#481](#481))
([c6f0137](c6f0137))
* **deps:** update dependency weaveworks/eksctl to v0.183.0
([#499](#499))
([9cb8e4d](9cb8e4d))
* **deps:** update grafana to 11.1.0
([#380](#380))
([499058a](499058a))
* **deps:** update istio to v1.22.2
([#512](#512))
([dcdadb4](dcdadb4))
* **deps:** update jest to v29.1.5
([#485](#485))
([9c392b9](9c392b9))
* **deps:** update neuvector to 5.3.3
([#467](#467))
([261057d](261057d))
* **deps:** update pepr to 0.32.2
([#473](#473))
([ab4bee9](ab4bee9))
* **deps:** update pepr to 0.32.3
([#494](#494))
([2e28897](2e28897))
* **deps:** update pepr to 0.32.6
([#516](#516))
([a9d3eec](a9d3eec))
* **deps:** update promtail to 3.1.0
([#335](#335))
([4457fce](4457fce))
* **deps:** update uds to v0.12.0
([#521](#521))
([8e587ff](8e587ff))
* **deps:** update uds-common tasks to 0.6.1
([#498](#498))
([4aa6e33](4aa6e33))
* **deps:** update zarf to v0.35.0
([#490](#490))
([86957cf](86957cf))
* docs linting changes
([#505](#505))
([0fe2015](0fe2015))
* remove emulated gitlab endpoints from keycloak
([#483](#483))
([495960c](495960c))
* update docs for group auth and readme for docs site
([#540](#540))
([ace7041](ace7041))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
rjferguson21 pushed a commit that referenced this pull request Jul 11, 2024
@mjnagel @jeff-mccoy
fix: operator retries and error logging (#511)
0b594c8 
## 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>
rjferguson21 pushed a commit that referenced this pull request Jul 11, 2024
@github-actions
chore(main): release 0.23.0 (#482)
cc68ac9 
🤖 I have created a release *beep* *boop*
---


##
[0.23.0](v0.22.2...v0.23.0)
(2024-07-04)


### ⚠ BREAKING CHANGES

* remove emulated gitlab endpoints from keycloak
([#483](#483))

### Features

* identity group auth
([#497](#497))
([d71d83e](d71d83e))


### Bug Fixes

* **docs:** re-ordered small paragraphs, clarified wording, and added
links to tech homepages
([#531](#531))
([6b2b46b](6b2b46b))
* **docs:** removed double-link which broke the markdown formatting in
pr template
([#532](#532))
([f41ced4](f41ced4))
* **docs:** uds-config.yaml example in k3d-slim-dev README
([#530](#530))
([2e1c53e](2e1c53e))
* operator retries and error logging
([#511](#511))
([cae5aab](cae5aab))


### Miscellaneous

* **deps:** update checkout action to latest sha
([#481](#481))
([c6f0137](c6f0137))
* **deps:** update dependency weaveworks/eksctl to v0.183.0
([#499](#499))
([9cb8e4d](9cb8e4d))
* **deps:** update grafana to 11.1.0
([#380](#380))
([499058a](499058a))
* **deps:** update istio to v1.22.2
([#512](#512))
([dcdadb4](dcdadb4))
* **deps:** update jest to v29.1.5
([#485](#485))
([9c392b9](9c392b9))
* **deps:** update neuvector to 5.3.3
([#467](#467))
([261057d](261057d))
* **deps:** update pepr to 0.32.2
([#473](#473))
([ab4bee9](ab4bee9))
* **deps:** update pepr to 0.32.3
([#494](#494))
([2e28897](2e28897))
* **deps:** update pepr to 0.32.6
([#516](#516))
([a9d3eec](a9d3eec))
* **deps:** update promtail to 3.1.0
([#335](#335))
([4457fce](4457fce))
* **deps:** update uds to v0.12.0
([#521](#521))
([8e587ff](8e587ff))
* **deps:** update uds-common tasks to 0.6.1
([#498](#498))
([4aa6e33](4aa6e33))
* **deps:** update zarf to v0.35.0
([#490](#490))
([86957cf](86957cf))
* docs linting changes
([#505](#505))
([0fe2015](0fe2015))
* remove emulated gitlab endpoints from keycloak
([#483](#483))
([495960c](495960c))
* update docs for group auth and readme for docs site
([#540](#540))
([ace7041](ace7041))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@blancharda blancharda mentioned this pull request Jul 12, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MxNxPx MxNxPx MxNxPx left review comments

@UnicornChance UnicornChance UnicornChance approved these changes

@rjferguson21 rjferguson21 rjferguson21 left review comments

@jeff-mccoy jeff-mccoy Awaiting requested review from jeff-mccoy

Assignees

@mjnagel mjnagel

Labels
None yet
Projects
None yet
Milestone
0.23.0
5 participants
@mjnagel @UnicornChance @rjferguson21 @MxNxPx @jeff-mccoy