Skip to content

Commit

Permalink
Merge pull request #4481 from fabriziopandini/fix-CoreDNS-1.20-1.21up…
Browse files Browse the repository at this point in the history
…grade

🐛 Fix CoreDNS upgrade from v1.20 to v1.21
  • Loading branch information
k8s-ci-robot authored Apr 15, 2021
2 parents c4082b2 + 92ac8b2 commit 6d7649f
Show file tree
Hide file tree
Showing 2 changed files with 161 additions and 17 deletions.
31 changes: 22 additions & 9 deletions controlplane/kubeadm/internal/workload_cluster_coredns.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ package internal
import (
"context"
"fmt"
"strings"

"github.com/blang/semver"

"github.com/coredns/corefile-migration/migration"
"github.com/pkg/errors"
Expand All @@ -39,6 +42,10 @@ const (
corefileBackupKey = "Corefile-backup"
coreDNSKey = "coredns"
coreDNSVolumeKey = "config-volume"

kubernetesImageRepository = "k8s.gcr.io"
oldCoreDNSImageName = "coredns"
coreDNSImageName = "coredns/coredns"
)

type coreDNSMigrator interface {
Expand Down Expand Up @@ -155,12 +162,12 @@ func (w *Workload) getCoreDNSInfo(ctx context.Context, clusterConfig *kubeadmv1.
}

// Handle imageRepository.
toImageRepository := fmt.Sprintf("%s/%s", parsedImage.Repository, parsedImage.Name)
toImageRepository := parsedImage.Repository
if clusterConfig.ImageRepository != "" {
toImageRepository = fmt.Sprintf("%s/%s", clusterConfig.ImageRepository, coreDNSKey)
toImageRepository = strings.TrimSuffix(clusterConfig.ImageRepository, "/")
}
if clusterConfig.DNS.ImageRepository != "" {
toImageRepository = fmt.Sprintf("%s/%s", clusterConfig.DNS.ImageRepository, coreDNSKey)
toImageRepository = strings.TrimSuffix(clusterConfig.DNS.ImageRepository, "/")
}

// Handle imageTag.
Expand All @@ -180,15 +187,21 @@ func (w *Workload) getCoreDNSInfo(ctx context.Context, clusterConfig *kubeadmv1.
return nil, err
}

// Handle the renaming of the upstream image from "k8s.gcr.io/coredns" to "k8s.gcr.io/coredns/coredns"
toImageName := parsedImage.Name
if toImageRepository == kubernetesImageRepository && toImageName == oldCoreDNSImageName && targetMajorMinorPatch.GTE(semver.MustParse("1.8.0")) {
toImageName = coreDNSImageName
}

return &coreDNSInfo{
Corefile: corefile,
Deployment: deployment,
CurrentMajorMinorPatch: currentMajorMinorPatch,
TargetMajorMinorPatch: targetMajorMinorPatch,
CurrentMajorMinorPatch: currentMajorMinorPatch.String(),
TargetMajorMinorPatch: targetMajorMinorPatch.String(),
FromImageTag: parsedImage.Tag,
ToImageTag: toImageTag,
FromImage: container.Image,
ToImage: fmt.Sprintf("%s:%s", toImageRepository, toImageTag),
ToImage: fmt.Sprintf("%s/%s:%s", toImageRepository, toImageName, toImageTag),
}, nil
}

Expand Down Expand Up @@ -297,12 +310,12 @@ func patchCoreDNSDeploymentImage(deployment *appsv1.Deployment, image string) {
}
}

func extractImageVersion(tag string) (string, error) {
func extractImageVersion(tag string) (semver.Version, error) {
ver, err := util.ParseMajorMinorPatch(tag)
if err != nil {
return "", err
return semver.Version{}, err
}
return fmt.Sprintf("%d.%d.%d", ver.Major, ver.Minor, ver.Patch), nil
return ver, nil
}

// validateCoreDNSImageTag returns error if the versions don't meet requirements.
Expand Down
147 changes: 139 additions & 8 deletions controlplane/kubeadm/internal/workload_cluster_coredns_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ func TestUpdateCoreDNS(t *testing.T) {
"BadCoreFileKey": "",
},
}
expectedImage := "k8s.gcr.io/some-folder/coredns:1.6.2"
depl := &appsv1.Deployment{
TypeMeta: v1.TypeMeta{
Kind: "Deployment",
Expand All @@ -83,13 +82,19 @@ func TestUpdateCoreDNS(t *testing.T) {
Spec: corev1.PodSpec{
Containers: []corev1.Container{{
Name: coreDNSKey,
Image: expectedImage,
Image: "k8s.gcr.io/some-folder/coredns:1.6.2",
}},
},
},
},
}

deplWithImage := func(image string) *appsv1.Deployment {
d := depl.DeepCopy()
d.Spec.Template.Spec.Containers[0].Image = image
return d
}

expectedCorefile := "coredns-core-file"
cm := &corev1.ConfigMap{
ObjectMeta: v1.ObjectMeta{
Expand Down Expand Up @@ -123,6 +128,7 @@ kind: ClusterConfiguration
objs []runtime.Object
expectErr bool
expectUpdates bool
expectImage string
}{
{
name: "returns early without error if skip core dns annotation is present",
Expand Down Expand Up @@ -280,6 +286,131 @@ kind: ClusterConfiguration
objs: []runtime.Object{depl, cm, kubeadmCM},
expectErr: false,
expectUpdates: true,
expectImage: "k8s.gcr.io/some-repo/coredns:1.7.2",
},
{
name: "updates everything successfully to v1.8.0 with a custom repo should not change the image name",
kcp: &controlplanev1.KubeadmControlPlane{
Spec: controlplanev1.KubeadmControlPlaneSpec{
KubeadmConfigSpec: cabpkv1.KubeadmConfigSpec{
ClusterConfiguration: &kubeadmv1.ClusterConfiguration{
DNS: kubeadmv1.DNS{
Type: kubeadmv1.CoreDNS,
ImageMeta: kubeadmv1.ImageMeta{
// provide an newer image to update to
ImageRepository: "k8s.gcr.io/some-repo",
ImageTag: "1.8.0",
},
},
},
},
},
},
migrator: &fakeMigrator{
migratedCorefile: "updated-core-file",
},
objs: []runtime.Object{deplWithImage("k8s.gcr.io/some-repo/coredns:1.7.0"), cm, kubeadmCM},
expectErr: false,
expectUpdates: true,
expectImage: "k8s.gcr.io/some-repo/coredns:1.8.0",
},
{
name: "kubeadm defaults, upgrade from Kubernetes v1.18.x to v1.19.y (from k8s.gcr.io/coredns:1.6.7 to k8s.gcr.io/coredns:1.7.0)",
kcp: &controlplanev1.KubeadmControlPlane{
Spec: controlplanev1.KubeadmControlPlaneSpec{
KubeadmConfigSpec: cabpkv1.KubeadmConfigSpec{
ClusterConfiguration: &kubeadmv1.ClusterConfiguration{
DNS: kubeadmv1.DNS{
Type: kubeadmv1.CoreDNS,
ImageMeta: kubeadmv1.ImageMeta{
ImageRepository: "k8s.gcr.io",
ImageTag: "1.7.0",
},
},
},
},
},
},
migrator: &fakeMigrator{
migratedCorefile: "updated-core-file",
},
objs: []runtime.Object{deplWithImage("k8s.gcr.io/coredns:1.6.7"), cm, kubeadmCM},
expectErr: false,
expectUpdates: true,
expectImage: "k8s.gcr.io/coredns:1.7.0",
},
{
name: "kubeadm defaults, upgrade from Kubernetes v1.19.x to v1.20.y (stay on k8s.gcr.io/coredns:1.7.0)",
kcp: &controlplanev1.KubeadmControlPlane{
Spec: controlplanev1.KubeadmControlPlaneSpec{
KubeadmConfigSpec: cabpkv1.KubeadmConfigSpec{
ClusterConfiguration: &kubeadmv1.ClusterConfiguration{
DNS: kubeadmv1.DNS{
Type: kubeadmv1.CoreDNS,
ImageMeta: kubeadmv1.ImageMeta{
ImageRepository: "k8s.gcr.io",
ImageTag: "1.7.0",
},
},
},
},
},
},
migrator: &fakeMigrator{
migratedCorefile: "updated-core-file",
},
objs: []runtime.Object{deplWithImage("k8s.gcr.io/coredns:1.7.0"), cm, kubeadmCM},
expectErr: false,
expectUpdates: false,
},
{
name: "kubeadm defaults, upgrade from Kubernetes v1.20.x to v1.21.y (from k8s.gcr.io/coredns:1.7.0 to k8s.gcr.io/coredns/coredns:v1.8.0)",
kcp: &controlplanev1.KubeadmControlPlane{
Spec: controlplanev1.KubeadmControlPlaneSpec{
KubeadmConfigSpec: cabpkv1.KubeadmConfigSpec{
ClusterConfiguration: &kubeadmv1.ClusterConfiguration{
DNS: kubeadmv1.DNS{
Type: kubeadmv1.CoreDNS,
ImageMeta: kubeadmv1.ImageMeta{
ImageRepository: "k8s.gcr.io",
ImageTag: "v1.8.0", // NOTE: ImageTags requires the v prefix
},
},
},
},
},
},
migrator: &fakeMigrator{
migratedCorefile: "updated-core-file",
},
objs: []runtime.Object{deplWithImage("k8s.gcr.io/coredns:1.7.0"), cm, kubeadmCM},
expectErr: false,
expectUpdates: true,
expectImage: "k8s.gcr.io/coredns/coredns:v1.8.0", // NOTE: ImageName has coredns/coredns
},
{
name: "kubeadm defaults, upgrade from Kubernetes v1.21.x to v1.22.y (stay on k8s.gcr.io/coredns/coredns:v1.8.0)",
kcp: &controlplanev1.KubeadmControlPlane{
Spec: controlplanev1.KubeadmControlPlaneSpec{
KubeadmConfigSpec: cabpkv1.KubeadmConfigSpec{
ClusterConfiguration: &kubeadmv1.ClusterConfiguration{
DNS: kubeadmv1.DNS{
Type: kubeadmv1.CoreDNS,
ImageMeta: kubeadmv1.ImageMeta{
ImageRepository: "k8s.gcr.io",
ImageTag: "v1.8.0", // NOTE: ImageTags requires the v prefix
},
},
},
},
},
},
migrator: &fakeMigrator{
migratedCorefile: "updated-core-file",
},
objs: []runtime.Object{deplWithImage("k8s.gcr.io/coredns/coredns:v1.8.0"), cm, kubeadmCM},
expectErr: false,
expectUpdates: false,
},
}

Expand All @@ -303,8 +434,8 @@ kind: ClusterConfiguration
// assert kubeadmConfigMap
var expectedKubeadmConfigMap corev1.ConfigMap
g.Expect(fakeClient.Get(context.TODO(), ctrlclient.ObjectKey{Name: kubeadmConfigKey, Namespace: metav1.NamespaceSystem}, &expectedKubeadmConfigMap)).To(Succeed())
g.Expect(expectedKubeadmConfigMap.Data).To(HaveKeyWithValue("ClusterConfiguration", ContainSubstring("1.7.2")))
g.Expect(expectedKubeadmConfigMap.Data).To(HaveKeyWithValue("ClusterConfiguration", ContainSubstring("k8s.gcr.io/some-repo")))
g.Expect(expectedKubeadmConfigMap.Data).To(HaveKeyWithValue("ClusterConfiguration", ContainSubstring(tt.kcp.Spec.KubeadmConfigSpec.ClusterConfiguration.DNS.ImageTag)))
g.Expect(expectedKubeadmConfigMap.Data).To(HaveKeyWithValue("ClusterConfiguration", ContainSubstring(tt.kcp.Spec.KubeadmConfigSpec.ClusterConfiguration.DNS.ImageRepository)))

// assert CoreDNS corefile
var expectedConfigMap corev1.ConfigMap
Expand All @@ -315,10 +446,10 @@ kind: ClusterConfiguration

// assert CoreDNS deployment
var actualDeployment appsv1.Deployment
g.Expect(fakeClient.Get(context.TODO(), ctrlclient.ObjectKey{Name: coreDNSKey, Namespace: metav1.NamespaceSystem}, &actualDeployment)).To(Succeed())
// ensure the image is updated and the volumes point to the corefile
g.Expect(actualDeployment.Spec.Template.Spec.Containers[0].Image).To(Equal("k8s.gcr.io/some-repo/coredns:1.7.2"))

g.Eventually(func() string {
g.Expect(fakeClient.Get(context.TODO(), ctrlclient.ObjectKey{Name: coreDNSKey, Namespace: metav1.NamespaceSystem}, &actualDeployment)).To(Succeed())
return actualDeployment.Spec.Template.Spec.Containers[0].Image
}, "5s").Should(Equal(tt.expectImage))
}
})
}
Expand Down

0 comments on commit 6d7649f

Please sign in to comment.