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

Create a manager cache only when a ns is specified: #156

Merged
merged 1 commit into from
Oct 2, 2023

Conversation

jacobweinstock
Copy link
Member

Description

A cache with a "" namespace causes reconcile errors:
"unable to get: namespace/machine because of unknown namespace for the cache"

With this change if the namespace is "" we use the default behavior of the manager which is to watch and list all namespaces.

I believe this was the behavior before we upgrade the controller runtime.

Why is this needed

Fixes: #

How Has This Been Tested?

How are existing users impacted? What migration steps/scripts do we need?

Checklist:

I have:

  • updated the documentation and/or roadmap (if required)
  • added unit or e2e tests
  • provided instructions on how to upgrade

A cache with a "" namespace causes reconcile errors:
"unable to get: namespace/machine because of unknown namespace for the cache"
With this change is the ns is "" we use the default behavior
of the manager which is to watch and list all namespaces.

Signed-off-by: Jacob Weinstock <[email protected]>
@jacobweinstock jacobweinstock added the kind/bug Categorizes issue or PR as related to a bug. label Sep 30, 2023
@codecov
Copy link

codecov bot commented Sep 30, 2023

Codecov Report

Merging #156 (c8ec298) into main (1065313) will not change coverage.
The diff coverage is n/a.

❗ Current head c8ec298 differs from pull request most recent head b24fff5. Consider uploading reports for the commit b24fff5 to get more accurate results

@@           Coverage Diff           @@
##             main     #156   +/-   ##
=======================================
  Coverage   63.17%   63.17%           
=======================================
  Files           5        5           
  Lines         478      478           
=======================================
  Hits          302      302           
  Misses        137      137           
  Partials       39       39           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@jacobweinstock jacobweinstock added the ready-to-merge Mergify: Ready for Merging label Oct 2, 2023
@mergify mergify bot merged commit 96dbc0e into tinkerbell:main Oct 2, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. ready-to-merge Mergify: Ready for Merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants