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

Removing unnecessary 'namespace' references to clear code and resolve error log messages #156

Merged
merged 4 commits into from
Feb 5, 2025

Conversation

tr-aheiev
Copy link

After changing (a long time ago) the scope of 'ClusterSecret' from namespaced to cluster-wide there was left few references to 'namespace' in code of operator and in test's files. This PR for removing those references and also for resolving errors like this:

[ERROR ] [dynamic-cluster-secret-match-namespaces] Handler 'on_field_match_namespace/matchNamespace' failed with an exception. Will retry. Traceback (most recent call last): File "/usr/local/lib/python3.9/site-packages/kopf/_core/actions/execution.py", line 276, in execute_handler_once result = await invoke_handler( File "/usr/local/lib/python3.9/site-packages/kopf/_core/actions/execution.py", line 371, in invoke_handler result = await invocation.invoke( File "/usr/local/lib/python3.9/site-packages/kopf/_core/actions/invocation.py", line 139, in invoke await asyncio.shield(future) # slightly expensive: creates tasks File "/usr/local/lib/python3.9/concurrent/futures/thread.py", line 58, in run result = self.fn(*self.args, **self.kwargs) File "/src/handlers.py", line 91, in on_field_match_namespace csecs_cache.set_cluster_secret(BaseClusterSecret( File "/usr/local/lib/python3.9/site-packages/pydantic/main.py", line 165, in __init__ __pydantic_self__.__pydantic_validator__.validate_python(data, self_instance=__pydantic_self__) pydantic_core._pydantic_core.ValidationError: 1 validation error for BaseClusterSecret namespace Input should be a valid string [type=string_type, input_value=None, input_type=NoneType] For further information visit https://errors.pydantic.dev/2.3/v/string_type

And same error is there: issuecomment-2090803901

After this PR all tests from 'src/tests' and from 'conformance' are passed.

There is similar PR for this purpose 154 , but I think my PR is more clear and resolve only 'namespace' problem (problem with 'on.field' for 'data' I will try to solve in another PR soon).

axel7083
axel7083 previously approved these changes Jan 21, 2025
Copy link
Collaborator

@axel7083 axel7083 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @tr-aheiev for your work, waiting on the PR checks to run

@tr-aheiev tr-aheiev force-pushed the fix-cs-namespaceless branch from 07f7686 to 8fc0cfe Compare January 21, 2025 15:32
@tr-aheiev
Copy link
Author

I rebased branch from my fork to last changes and now it must pass workflow tests without issues

@peterjenkins1
Copy link
Contributor

Hey, any reason why this isn't merged? It's kind of hard to debug other issues when the logs are full of this error.

@axel7083
Copy link
Collaborator

axel7083 commented Feb 5, 2025

Hey @peterjenkins1 @tr-aheiev sme e2e tests seems to be failing. Need a version bump, at least patch 0.5.1

cluster-secret => (version: "0.5.0", path: "charts/cluster-secret") > chart version not ok. Needs a version bump! 

@peterjenkins1
Copy link
Contributor

Ok, that's easy enough to fix. I assumed the versioning was handled by some automation based on GitHub PR tags I couldn't see.

Let's see if I am able to do that myself...

@peterjenkins1
Copy link
Contributor

@axel7083 is there an easier way to make this small change besides me making a new fork and a new PR?

Copy link
Collaborator

@axel7083 axel7083 left a comment

Choose a reason for hiding this comment

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

LGTM

@axel7083 axel7083 merged commit 98fcce1 into zakkg3:master Feb 5, 2025
14 checks passed
@axel7083
Copy link
Collaborator

axel7083 commented Feb 5, 2025

@peterjenkins1 @tr-aheiev new release available 0.5.1

@peterjenkins1
Copy link
Contributor

Thank you!

@peterjenkins1
Copy link
Contributor

@axel7083 I'm testing the new chart release, but I don't see a newer version of the image. It looks like the image hasn't been released in 4 months and the version in the Helm chart is still 0.0.12. Is that intentional?

@axel7083
Copy link
Collaborator

axel7083 commented Feb 5, 2025

@axel7083 I'm testing the new chart release, but I don't see a newer version of the image. It looks like the image hasn't been released in 4 months and the version in the Helm chart is still 0.0.12. Is that intentional?

I just published the 0.0.14 image https://quay.io/repository/clustersecret/clustersecret?tab=tags (forgot that I have the make a tag on the branch)

@peterjenkins1
Copy link
Contributor

Thanks again @axel7083

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants