-
Notifications
You must be signed in to change notification settings - Fork 462
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
Harmonize Applier, ChartRenderer and ChartApplier creation #2417
Conversation
0e1f07e
to
8bbd667
Compare
8bbd667
to
6569e64
Compare
6569e64
to
946196b
Compare
removed commit from #2415 /ready-for-review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice PR... no comments from my side, other than:
/lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some minor stuff. Otherwise looks good.
946196b
to
df041a8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
@plkokanov Command |
How to categorize this PR?
/area quality
/kind enhancement
/priority normal
What this PR does / why we need it:
This PR harmonizes/centralizes how
Applier
,ChartRenderer
andChartApplier
instances are obtained in g/g.It removes any seperate constructions of
ChartRenderer
s andChartApplier
s in favor of two respective functions that were added topkg/client/kubernetes.Interface
to obtain such instances.This is needed to ensure, that we use the same controller-runtime clients while applying Charts everywhere to be able to leverage the clients' caches (which is not possible, if we construct new clients,
ChartApplier
s and so on, wherever needed).Which issue(s) this PR fixes:
Part of #2414
Special notes for your reviewer:
This PR also contains one commit from #2415, as it depends on it.
I opened this as a draft PR to collect early feedback and will rebase and drop the commit once #2415 has been merged.
/invite @gardener/gardener-maintainers
✅ Depends on #2415
Release note: