Upgrade registry certificate in packages #7881
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
eks-distro-bot
added
the
size/L
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7881 +/- ##
==========================================
+ Coverage 73.39% 73.49% +0.10%
==========================================
Files 576 578 +2
Lines 35622 35790 +168
==========================================
+ Hits 26144 26305 +161
- Misses 7826 7829 +3
- Partials 1652 1656 +4 ☔ View full report in Codecov by Sentry. |
|
@tatlat: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
| // UpgradeCuratedPackages upgrades curated packages as part of the cluster upgrade. | ||
| func (pi *Installer) UpgradeCuratedPackages(ctx context.Context) { | ||
| if IsPackageControllerDisabled(pi.spec.Cluster) { | ||
| logger.Info(" Package controller disabled") |
There was a problem hiding this comment.
why the space?
| logger.Info(" Package controller disabled") | |
| logger.Info("Package controller disabled") |
| err := pi.installPackagesController(ctx) | ||
|
|
||
| if err != nil { |
There was a problem hiding this comment.
nit
| err := pi.installPackagesController(ctx) | |
| if err != nil { | |
| if err := pi.installPackagesController(ctx); err != nil { |
| &bundle.PackageController.HelmChart, | ||
| f.registryMirror, | ||
|
|
||
| opts = append(opts, |
There was a problem hiding this comment.
nit: I would expect to be able to override these defaults with the new variadic args, but this makes it impossible
| @@ -76,6 +76,7 @@ type EksdUpgrader interface { | |||
|
|
|||
| type PackageInstaller interface { | |||
| InstallCuratedPackages(ctx context.Context) | |||
| UpgradeCuratedPackages(ctx context.Context) | |||
There was a problem hiding this comment.
could we have a different interface? or change the name of the interface
I prefer a different interface, but up to you
Issue #, if available:
Description of changes:
Testing (if applicable):
Documentation added/planned (if applicable):
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.