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

Saas detection error handling #493

Merged
merged 4 commits into from
Feb 9, 2024
Merged

Saas detection error handling #493

merged 4 commits into from
Feb 9, 2024

Conversation

ChrisKujawa
Copy link
Member

Paired with @oleschoenburg on this:

  • We agreed on to "simply" pass the error to the front, to the respective commands so they can either fail or do what ever
  • For errors indicating resource not-found we decided to return false, and use SM setting (as we expect this CRD not in the SM env)
  • We wrote some more tests for the error handling
  • I had to refactor a bit the client creation, so we test the right things here

closes #488
closes #490

@ChrisKujawa
fix: return error
24c580c 
Instead of assuming SM on all errors, we check for NotFound type
and if this is not equal we return the respective error
@ChrisKujawa
test: verify error handling
82dee4a
@ChrisKujawa
refactor: adjust usage
76e0aee 
correctly handle returned error
@ChrisKujawa
refactor: extract client creation
1de84b6 
Extract the client creation from the checking of the environment.
Use the internal client creation in the tests, which are checking actual configurations changes
here we don't need any SaaS/SM differentiation
lenaschoenburg
lenaschoenburg approved these changes Feb 9, 2024
View reviewed changes
Copy link
Member

@lenaschoenburg lenaschoenburg left a comment

Choose a reason for hiding this comment

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

Thanks 😍

go-chaos/internal/k8helper.go Show resolved Hide resolved
@ChrisKujawa
Copy link
Member Author 

go build -o zbchaos main.go
# github.com/zeebe-io/zeebe-chaos/go-chaos/internal
Error: internal/k8helper.go:5[9](https://github.com/zeebe-io/zeebe-chaos/actions/runs/7843937019/job/21405291067#step:7:10):[10](https://github.com/zeebe-io/zeebe-chaos/actions/runs/7843937019/job/21405291067#step:7:11): cannot use nil as K8Client value in return statement
make: *** [Makefile:11: build] Error 1
Error: Process completed with exit code 2.

😿

@ChrisKujawa
Copy link
Member Author

@oleschoenburg I would remove the last commit, we would either have to return K8Client{} or simply the client obj we had before. wdyt?

@lenaschoenburg
Copy link
Member

Ah how annoying. Yeah, just drop the last commit.

@ChrisKujawa ChrisKujawa force-pushed the ck-saas-detection-error-handling branch from 1a6763f to 1de84b6 Compare February 9, 2024 14:37
@ChrisKujawa ChrisKujawa merged commit cc9c847 into main Feb 9, 2024
5 checks passed
@ChrisKujawa ChrisKujawa deleted the ck-saas-detection-error-handling branch February 9, 2024 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lenaschoenburg lenaschoenburg lenaschoenburg approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Detection of SaaS vs SM environment incorrectly fallback to SM
2 participants
@ChrisKujawa @lenaschoenburg