fix: namespace was not found but working #468
Conversation
|
Warning Rate limit exceeded@mojtaba-esk has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 53 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe primary change in Changes
Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- pkg/k8s/k8s_namespace.go (1 hunks)
- pkg/k8s/k8s_namespace_test.go (6 hunks)
Additional comments not posted (8)
pkg/k8s/k8s_namespace.go (4)
Line range hint
8-15: LGTM!The
CreateNamespacefunction correctly handles the creation of a namespace and accounts for the case where the namespace already exists.
Line range hint
17-22: LGTM!The
DeleteNamespacefunction correctly handles the deletion of a namespace and wraps errors appropriately.
Line range hint
24-24: LGTM!The
GetNamespacefunction correctly returns the namespace using the Kubernetes client.
42-54: LGTM!The
NamespaceExistsfunction correctly checks for the existence of a namespace and logs different error conditions appropriately.pkg/k8s/k8s_namespace_test.go (4)
Line range hint
11-46: LGTM!The
TestCreateNamespacefunction correctly tests various scenarios for namespace creation, including successful creation, namespace already existing, and client errors.
Line range hint
48-82: LGTM!The
TestDeleteNamespacefunction correctly tests various scenarios for namespace deletion, including successful deletion, namespace not found, and client errors.
Line range hint
130-187: LGTM!The
TestGetNamespacefunction correctly tests various scenarios for namespace retrieval, including successful retrieval, namespace not found, and client errors. The use of theassertErrfunction improves error handling and assertions.
Line range hint
189-218: LGTM!The
TestNamespaceExistsfunction correctly tests various scenarios for checking namespace existence, including namespace existing, namespace not existing, and client errors.
There was a problem hiding this comment.
For the future, when the issue itself doesn't have much info, please add some more context to the PR description around the reason for the bug and update logic change to help with the review.
From my review I'm assuming the issue was that all errors were being treated as the namespace not existing and this PR separates those error conditions.
If my assumption is wrong, please lmk.
The issue was when the namespace does not exist, the error was wrapped and could not be matched using k8s api error package therefore it returned the error. It was working, but it was showing that error. |
There was a problem hiding this comment.
Actionable comments posted: 4
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- pkg/k8s/k8s_namespace.go (1 hunks)
- pkg/k8s/k8s_namespace_test.go (5 hunks)
Files skipped from review as they are similar to previous changes (1)
- pkg/k8s/k8s_namespace_test.go
Additional context used
GitHub Check: test (./e2e/bittwister, 60m)
pkg/k8s/k8s_namespace.go
[failure] 46-46:
not enough return values
[failure] 49-49:
undefined: errors
[failure] 51-51:
not enough return values
[failure] 55-55:
not enough return values
GitHub Check: test (./e2e/system, 15m)
pkg/k8s/k8s_namespace.go
[failure] 46-46:
not enough return values
[failure] 49-49:
undefined: errors
[failure] 51-51:
not enough return values
[failure] 55-55:
not enough return values
GitHub Check: Run govulncheck
pkg/k8s/k8s_namespace.go
[failure] 46-46:
not enough return values
[failure] 49-49:
undefined: errors
[failure] 51-51:
not enough return values
[failure] 55-55:
not enough return values
GitHub Check: test (./e2e/basic, 15m)
pkg/k8s/k8s_namespace.go
[failure] 46-46:
not enough return values
[failure] 49-49:
undefined: errors
[failure] 51-51:
not enough return values
[failure] 55-55:
not enough return values
GitHub Check: test (./pkg/..., 10m)
pkg/k8s/k8s_namespace.go
[failure] 46-46:
not enough return values
[failure] 49-49:
undefined: errors
[failure] 51-51:
not enough return values
[failure] 55-55:
not enough return values
[failure] 46-46:
not enough return values
[failure] 49-49:
undefined: errors
[failure] 51-51:
not enough return values
[failure] 55-55:
not enough return values
GitHub Check: golangci-lint
pkg/k8s/k8s_namespace.go
[failure] 46-46:
not enough return values
[failure] 49-49:
undefined: errors
[failure] 51-51:
not enough return values
[failure] 55-55:
not enough return values
[failure] 46-46:
not enough return values
[failure] 49-49:
undefined: errors
[failure] 51-51:
not enough return values
[failure] 55-55:
not enough return values
Additional comments not posted (1)
pkg/k8s/k8s_namespace.go (1)
Line range hint
38-40:
LGTM!The simplification of the
GetNamespacefunction to directly return the result of the client call is appropriate and aligns with the PR objectives.Tools
GitHub Check: test (./e2e/bittwister, 60m)
[failure] 46-46:
not enough return values
[failure] 49-49:
undefined: errors
[failure] 51-51:
not enough return values
[failure] 55-55:
not enough return valuesGitHub Check: test (./e2e/system, 15m)
[failure] 46-46:
not enough return values
[failure] 49-49:
undefined: errors
[failure] 51-51:
not enough return values
[failure] 55-55:
not enough return valuesGitHub Check: Run govulncheck
[failure] 46-46:
not enough return values
[failure] 49-49:
undefined: errors
[failure] 51-51:
not enough return values
[failure] 55-55:
not enough return valuesGitHub Check: test (./e2e/basic, 15m)
[failure] 46-46:
not enough return values
[failure] 49-49:
undefined: errors
[failure] 51-51:
not enough return values
[failure] 55-55:
not enough return valuesGitHub Check: test (./pkg/..., 10m)
[failure] 46-46:
not enough return values
[failure] 49-49:
undefined: errors
[failure] 51-51:
not enough return values
[failure] 55-55:
not enough return values
[failure] 46-46:
not enough return values
[failure] 49-49:
undefined: errors
[failure] 51-51:
not enough return values
[failure] 55-55:
not enough return valuesGitHub Check: golangci-lint
[failure] 46-46:
not enough return values
[failure] 49-49:
undefined: errors
[failure] 51-51:
not enough return values
[failure] 55-55:
not enough return values
[failure] 46-46:
not enough return values
[failure] 49-49:
undefined: errors
[failure] 51-51:
not enough return values
[failure] 55-55:
not enough return values
Closes #463
Summary by CodeRabbit
Bug Fixes
Tests