fix image related bugs when enabling addons

pull/15984/head
Steven Powell 2023-03-06 14:19:18 -08:00
parent b4e93fc231
commit 93a8607044
3 changed files with 231 additions and 12 deletions

View File

@ -66,8 +66,12 @@ You can view the list of minikube maintainers at: https://github.com/kubernetes/
}
}
}
viper.Set(config.AddonImages, images)
viper.Set(config.AddonRegistries, registries)
if images != "" {
viper.Set(config.AddonImages, images)
}
if registries != "" {
viper.Set(config.AddonRegistries, registries)
}
err := addons.SetAndSave(ClusterFlagValue(), addon, "true")
if err != nil && !errors.Is(err, addons.ErrSkipThisAddon) {
exit.Error(reason.InternalAddonEnable, "enable failed", err)

View File

@ -815,10 +815,11 @@ func SelectAndPersistImages(addon *Addon, cc *config.ClusterConfig) (images, cus
}
if _, ok := addonDefaultImages[name]; !ok {
out.WarningT("Ignoring unknown custom image {{.name}}", out.V{"name": name})
delete(newImages, name)
}
}
// Use newly configured custom images.
images = overrideDefaults(addonDefaultImages, newImages)
images = overrideDefaults(images, newImages)
// Store custom addon images to be written.
cc.CustomAddonImages = mergeMaps(cc.CustomAddonImages, newImages)
}
@ -827,19 +828,17 @@ func SelectAndPersistImages(addon *Addon, cc *config.ClusterConfig) (images, cus
customRegistries = filterKeySpace(addonDefaultImages, cc.CustomAddonRegistries) // filter by images map because registry map may omit default registry.
if viper.IsSet(config.AddonRegistries) {
// Parse the AddonRegistries flag if present.
customRegistries = parseMapString(viper.GetString(config.AddonRegistries))
for name := range customRegistries {
newRegistries := parseMapString(viper.GetString(config.AddonRegistries))
for name := range newRegistries {
if _, ok := addonDefaultImages[name]; !ok { // check images map because registry map may omitted default registry
out.WarningT("Ignoring unknown custom registry {{.name}}", out.V{"name": name})
delete(customRegistries, name)
delete(newRegistries, name)
}
}
// Since registry map may omit default registry, any previously set custom registries for these images must be cleared.
for name := range addonDefaultImages {
delete(cc.CustomAddonRegistries, name)
}
// Use newly configured custom registries.
customRegistries = mergeMaps(customRegistries, newRegistries)
// Merge newly set registries into custom addon registries to be written.
cc.CustomAddonRegistries = mergeMaps(cc.CustomAddonRegistries, customRegistries)
cc.CustomAddonRegistries = mergeMaps(cc.CustomAddonRegistries, newRegistries)
}
// If images or registries were specified, save the config afterward.

View File

@ -16,7 +16,13 @@ limitations under the License.
package assets
import "testing"
import (
"fmt"
"testing"
"github.com/spf13/viper"
"k8s.io/minikube/pkg/minikube/config"
)
// mapsEqual returns true if and only if `a` contains all the same pairs as `b`.
func mapsEqual(a, b map[string]string) bool {
@ -142,3 +148,213 @@ func TestOverrideDefautls(t *testing.T) {
}
}
}
func TestSelectAndPersistImages(t *testing.T) {
gcpAuth := Addons["gcp-auth"]
gcpAuthImages := gcpAuth.Images
type expected struct {
numImages int
numRegistries int
numCustomImages int
numCustomRegistries int
}
test := func(t *testing.T, cc *config.ClusterConfig, e expected) (images, registries map[string]string) {
images, registries, err := SelectAndPersistImages(gcpAuth, cc)
if err != nil {
t.Fatal(err)
}
if len(images) != e.numImages {
t.Errorf("expected %d images but got %v", e.numImages, images)
}
if len(registries) != e.numRegistries {
t.Errorf("expected %d registries but got %v", e.numRegistries, registries)
}
if len(cc.CustomAddonImages) != e.numCustomImages {
t.Errorf("expected %d CustomAddonImages in config but got %+v", e.numCustomImages, cc.CustomAddonImages)
}
if len(cc.CustomAddonRegistries) != e.numCustomRegistries {
t.Errorf("expected %d CustomAddonRegistries in config but got %+v", e.numCustomRegistries, cc.CustomAddonRegistries)
}
return images, registries
}
t.Run("NoCustomImage", func(t *testing.T) {
cc := &config.ClusterConfig{}
e := expected{numImages: 2}
images, _ := test(t, cc, e)
checkMatches(t, "KubeWebhookCertgen", images, gcpAuthImages)
checkMatches(t, "GCPAuthWebhook", images, gcpAuthImages)
})
t.Run("ExistingCustomImage", func(t *testing.T) {
cc := &config.ClusterConfig{
CustomAddonImages: map[string]string{
"GCPAuthWebhook": "test123",
},
}
e := expected{numImages: 2, numCustomImages: 1}
images, _ := test(t, cc, e)
checkMatches(t, "KubeWebhookCertgen", images, gcpAuthImages)
checkMatches(t, "GCPAuthWebhook", images, cc.CustomAddonImages)
})
t.Run("NewCustomImage", func(t *testing.T) {
cc := &config.ClusterConfig{}
e := expected{numImages: 2, numCustomImages: 1}
addonImages := setAddonImages("GCPAuthWebhook", "test123")
defer viper.Reset()
images, _ := test(t, cc, e)
checkMatches(t, "KubeWebhookCertgen", images, gcpAuthImages)
checkMatches(t, "GCPAuthWebhook", images, addonImages)
})
t.Run("NewAndExistingCustomImages", func(t *testing.T) {
cc := &config.ClusterConfig{
CustomAddonImages: map[string]string{
"GCPAuthWebhook": "test123",
},
}
e := expected{numImages: 2, numCustomImages: 2}
addonImages := setAddonImages("KubeWebhookCertgen", "test456")
defer viper.Reset()
images, _ := test(t, cc, e)
checkMatches(t, "KubeWebhookCertgen", images, addonImages)
checkMatches(t, "GCPAuthWebhook", images, cc.CustomAddonImages)
})
t.Run("NewOverwritesExistingCustomImage", func(t *testing.T) {
cc := &config.ClusterConfig{
CustomAddonImages: map[string]string{
"GCPAuthWebhook": "test123",
},
}
e := expected{numImages: 2, numCustomImages: 1}
addonImages := setAddonImages("GCPAuthWebhook", "test456")
defer viper.Reset()
images, _ := test(t, cc, e)
checkMatches(t, "KubeWebhookCertgen", images, gcpAuthImages)
checkMatches(t, "GCPAuthWebhook", images, addonImages)
})
t.Run("NewUnrelatedCustomImageWithExistingCustomImage", func(t *testing.T) {
cc := &config.ClusterConfig{
CustomAddonImages: map[string]string{
"GCPAuthWebhook": "test123",
},
}
e := expected{numImages: 2, numCustomImages: 1}
setAddonImages("IngressDNS", "test456")
defer viper.Reset()
images, _ := test(t, cc, e)
checkMatches(t, "KubeWebhookCertgen", images, gcpAuthImages)
checkMatches(t, "GCPAuthWebhook", images, cc.CustomAddonImages)
})
t.Run("NewCustomImageWithExistingUnrelatedCustomImage", func(t *testing.T) {
cc := &config.ClusterConfig{
CustomAddonImages: map[string]string{
"IngressDNS": "test123",
},
}
e := expected{numImages: 2, numCustomImages: 2}
addonImages := setAddonImages("GCPAuthWebhook", "test456")
defer viper.Reset()
images, _ := test(t, cc, e)
checkMatches(t, "KubeWebhookCertgen", images, gcpAuthImages)
checkMatches(t, "GCPAuthWebhook", images, addonImages)
})
t.Run("ExistingCustomRegistry", func(t *testing.T) {
cc := &config.ClusterConfig{
CustomAddonRegistries: map[string]string{
"GCPAuthWebhook": "test123",
},
}
e := expected{numImages: 2, numRegistries: 1, numCustomRegistries: 1}
_, registries := test(t, cc, e)
checkMatches(t, "GCPAuthWebhook", registries, cc.CustomAddonRegistries)
})
t.Run("NewCustomRegistry", func(t *testing.T) {
cc := &config.ClusterConfig{}
e := expected{numImages: 2, numRegistries: 1, numCustomRegistries: 1}
addonRegistries := setAddonRegistries("GCPAuthWebhook", "test123")
defer viper.Reset()
_, registries := test(t, cc, e)
checkMatches(t, "GCPAuthWebhook", registries, addonRegistries)
})
t.Run("NewAndExistingCustomRegistries", func(t *testing.T) {
cc := &config.ClusterConfig{
CustomAddonRegistries: map[string]string{
"GCPAuthWebhook": "test123",
},
}
e := expected{numImages: 2, numRegistries: 2, numCustomRegistries: 2}
addonRegistries := setAddonRegistries("KubeWebhookCertgen", "test456")
defer viper.Reset()
_, registries := test(t, cc, e)
checkMatches(t, "GCPAuthWebhook", registries, cc.CustomAddonRegistries)
checkMatches(t, "KubeWebhookCertgen", registries, addonRegistries)
})
t.Run("NewOverwritesExistingCustomRegistry", func(t *testing.T) {
cc := &config.ClusterConfig{
CustomAddonRegistries: map[string]string{
"GCPAuthWebhook": "test123",
},
}
e := expected{numImages: 2, numRegistries: 1, numCustomRegistries: 1}
addonRegistries := setAddonRegistries("GCPAuthWebhook", "test456")
defer viper.Reset()
_, registries := test(t, cc, e)
checkMatches(t, "GCPAuthWebhook", registries, addonRegistries)
})
t.Run("NewUnrelatedCustomRegistryWithExistingCustomRegistry", func(t *testing.T) {
cc := &config.ClusterConfig{
CustomAddonRegistries: map[string]string{
"GCPAuthWebhook": "test123",
},
}
e := expected{numImages: 2, numRegistries: 1, numCustomRegistries: 1}
setAddonRegistries("IngressDNS", "test456")
defer viper.Reset()
_, registries := test(t, cc, e)
checkMatches(t, "GCPAuthWebhook", registries, cc.CustomAddonRegistries)
})
t.Run("NewCustomRegistryWithExistingUnrelatedCustomRegistry", func(t *testing.T) {
cc := &config.ClusterConfig{
CustomAddonRegistries: map[string]string{
"IngressDNS": "test123",
},
}
e := expected{numImages: 2, numRegistries: 1, numCustomRegistries: 2}
addonRegistries := setAddonRegistries("GCPAuthWebhook", "test456")
defer viper.Reset()
_, registries := test(t, cc, e)
checkMatches(t, "GCPAuthWebhook", registries, addonRegistries)
})
}
func setAddonImages(k, v string) map[string]string {
return setFlag(config.AddonImages, k, v)
}
func setAddonRegistries(k, v string) map[string]string {
return setFlag(config.AddonRegistries, k, v)
}
func setFlag(name, k, v string) map[string]string {
viper.Set(name, fmt.Sprintf("%s=%s", k, v))
return map[string]string{k: v}
}
func checkMatches(t *testing.T, name string, got, expected map[string]string) {
if expected[name] != got[name] {
t.Errorf("expected %q to be %q, but got %q", name, expected[name], got[name])
}
}