fix migration logic

modify addon test to cover the entire migration logic

cmd/kubeadm/app/phases/addons/dns/dns.go

@@ -279,7 +279,13 @@ func createCoreDNSAddon(deploymentBytes, serviceBytes, configBytes []byte, clien
 	// Assume that migration is always possible, rely on migrateCoreDNSCorefile() to fail if not.
 	canMigrateCorefile := true
 
-	if corefileMigrationRequired {
+	if corefile == "" || migration.Default("", corefile) {
+		// If the Corefile is empty or default, the latest default Corefile will be applied
+		if err := apiclient.CreateOrUpdateConfigMap(client, coreDNSConfigMap); err != nil {
+			return err
+		}
+	} else if corefileMigrationRequired {
+		// If migration is required, try and migrate the Corefile
 		if err := migrateCoreDNSCorefile(client, coreDNSConfigMap, corefile, currentInstalledCoreDNSVersion); err != nil {
 			// Errors in Corefile Migration is verified during preflight checks. This part will be executed when a user has chosen
 			// to ignore preflight check errors.
@@ -290,7 +296,8 @@ func createCoreDNSAddon(deploymentBytes, serviceBytes, configBytes []byte, clien
 			}
 		}
 	} else {
-		if err := apiclient.CreateOrUpdateConfigMap(client, coreDNSConfigMap); err != nil {
+		// If the Corefile is modified and doesn't require any migration, it'll be retained for the benefit of the user
+		if err := apiclient.CreateOrRetainConfigMap(client, coreDNSConfigMap, kubeadmconstants.CoreDNSConfigMap); err != nil {
 			return err
 		}
 	}
@@ -370,7 +377,9 @@ func createDNSService(dnsService *v1.Service, serviceBytes []byte, client client
 // isCoreDNSConfigMapMigrationRequired checks if a migration of the CoreDNS ConfigMap is required.
 func isCoreDNSConfigMapMigrationRequired(corefile, currentInstalledCoreDNSVersion string) (bool, error) {
 	var isMigrationRequired bool
-	if corefile == "" || migration.Default("", corefile) {
+
+	// Current installed version is expected to be empty for init
+	if currentInstalledCoreDNSVersion == "" {
 		return isMigrationRequired, nil
 	}
 	deprecated, err := migration.Deprecated(currentInstalledCoreDNSVersion, kubeadmconstants.CoreDNSVersion, corefile)

cmd/kubeadm/app/phases/addons/dns/dns_test.go

@@ -553,17 +553,174 @@ func TestDeploymentsHaveSystemClusterCriticalPriorityClassName(t *testing.T) {
 	}
 }
 
-func TestCreateCoreDNSConfigMap(t *testing.T) {
+func TestCreateCoreDNSAddon(t *testing.T) {
 	tests := []struct {
 		name                 string
 		initialCorefileData  string
 		expectedCorefileData string
 		coreDNSVersion       string
 	}{
+		{
+			name:                "Empty Corefile",
+			initialCorefileData: "",
+			expectedCorefileData: `.:53 {
+    errors
+    health {
+       lameduck 5s
+    }
+    ready
+    kubernetes cluster.local in-addr.arpa ip6.arpa {
+       pods insecure
+       fallthrough in-addr.arpa ip6.arpa
+       ttl 30
+    }
+    prometheus :9153
+    forward . /etc/resolv.conf {
+       max_concurrent 1000
+    }
+    cache 30
+    loop
+    reload
+    loadbalance
+}
+`,
+			coreDNSVersion: "1.6.7",
+		},
+		{
+			name: "Default Corefile",
+			initialCorefileData: `.:53 {
+        errors
+        health {
+            lameduck 5s
+        }
+        ready
+        kubernetes cluster.local in-addr.arpa ip6.arpa {
+            pods insecure
+            fallthrough in-addr.arpa ip6.arpa
+            ttl 30
+        }
+        prometheus :9153
+        forward . /etc/resolv.conf
+        cache 30
+        loop
+        reload
+        loadbalance
+    }
+`,
+			expectedCorefileData: `.:53 {
+    errors
+    health {
+       lameduck 5s
+    }
+    ready
+    kubernetes cluster.local in-addr.arpa ip6.arpa {
+       pods insecure
+       fallthrough in-addr.arpa ip6.arpa
+       ttl 30
+    }
+    prometheus :9153
+    forward . /etc/resolv.conf {
+       max_concurrent 1000
+    }
+    cache 30
+    loop
+    reload
+    loadbalance
+}
+`,
+			coreDNSVersion: "1.6.7",
+		},
+		{
+			name: "Modified Corefile with only newdefaults needed",
+			initialCorefileData: `.:53 {
+        errors
+        log
+        health
+        ready
+        kubernetes cluster.local in-addr.arpa ip6.arpa {
+            pods insecure
+            fallthrough in-addr.arpa ip6.arpa
+            ttl 30
+        }
+        prometheus :9153
+        forward . /etc/resolv.conf
+        cache 30
+        loop
+        reload
+        loadbalance
+    }
+`,
+			expectedCorefileData: `.:53 {
+    errors
+    log
+    health {
+        lameduck 5s
+    }
+    ready
+    kubernetes cluster.local in-addr.arpa ip6.arpa {
+        pods insecure
+        fallthrough in-addr.arpa ip6.arpa
+        ttl 30
+    }
+    prometheus :9153
+    forward . /etc/resolv.conf {
+        max_concurrent 1000
+    }
+    cache 30
+    loop
+    reload
+    loadbalance
+}
+`,
+			coreDNSVersion: "1.6.2",
+		},
+		{
+			name: "Default Corefile with rearranged plugins",
+			initialCorefileData: `.:53 {
+        errors
+        cache 30
+        prometheus :9153
+        forward . /etc/resolv.conf
+        loop
+        reload
+        loadbalance
+        kubernetes cluster.local in-addr.arpa ip6.arpa {
+           pods insecure
+           upstream
+           fallthrough in-addr.arpa ip6.arpa
+           ttl 30
+        }
+        health
+    }
+`,
+			expectedCorefileData: `.:53 {
+    errors
+    health {
+       lameduck 5s
+    }
+    ready
+    kubernetes cluster.local in-addr.arpa ip6.arpa {
+       pods insecure
+       fallthrough in-addr.arpa ip6.arpa
+       ttl 30
+    }
+    prometheus :9153
+    forward . /etc/resolv.conf {
+       max_concurrent 1000
+    }
+    cache 30
+    loop
+    reload
+    loadbalance
+}
+`,
+			coreDNSVersion: "1.3.1",
+		},
 		{
 			name: "Remove Deprecated options",
 			initialCorefileData: `.:53 {
         errors
+        logs
         health
         kubernetes cluster.local in-addr.arpa ip6.arpa {
            pods insecure
@@ -580,6 +737,7 @@ func TestCreateCoreDNSConfigMap(t *testing.T) {
     }`,
 			expectedCorefileData: `.:53 {
     errors
+    logs
     health {
         lameduck 5s
     }
@@ -642,19 +800,106 @@ func TestCreateCoreDNSConfigMap(t *testing.T) {
 `,
 			coreDNSVersion: "1.3.1",
 		},
+		{
+			name: "Modified Corefile with no migration required",
+			initialCorefileData: `consul {
+        errors
+        forward . 10.10.96.16:8600 10.10.96.17:8600 10.10.96.18:8600 {
+            max_concurrent 1000
+        }
+        loadbalance
+        cache 5
+        reload
+    }
+    domain.int {
+       errors
+       forward . 10.10.0.140 10.10.0.240 10.10.51.40 {
+           max_concurrent 1000
+       }
+       loadbalance
+       cache 3600
+       reload
+    }
+    .:53 {
+      errors
+      health {
+          lameduck 5s
+      }
+      ready
+      kubernetes cluster.local in-addr.arpa ip6.arpa {
+          pods insecure
+          fallthrough in-addr.arpa ip6.arpa
+      }
+      prometheus :9153
+      forward . /etc/resolv.conf {
+          prefer_udp
+          max_concurrent 1000
+      }
+      cache 30
+      loop
+      reload
+      loadbalance
+    }
+`,
+			expectedCorefileData: `consul {
+        errors
+        forward . 10.10.96.16:8600 10.10.96.17:8600 10.10.96.18:8600 {
+            max_concurrent 1000
+        }
+        loadbalance
+        cache 5
+        reload
+    }
+    domain.int {
+       errors
+       forward . 10.10.0.140 10.10.0.240 10.10.51.40 {
+           max_concurrent 1000
+       }
+       loadbalance
+       cache 3600
+       reload
+    }
+    .:53 {
+      errors
+      health {
+          lameduck 5s
+      }
+      ready
+      kubernetes cluster.local in-addr.arpa ip6.arpa {
+          pods insecure
+          fallthrough in-addr.arpa ip6.arpa
+      }
+      prometheus :9153
+      forward . /etc/resolv.conf {
+          prefer_udp
+          max_concurrent 1000
+      }
+      cache 30
+      loop
+      reload
+      loadbalance
+    }
+`,
+			coreDNSVersion: "1.6.7",
+		},
 	}
 
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
 			client := createClientAndCoreDNSManifest(t, tc.initialCorefileData, tc.coreDNSVersion)
-			// Get the Corefile and installed CoreDNS version.
+
-			cm, corefile, currentInstalledCoreDNSVersion, err := GetCoreDNSInfo(client)
+			configMapBytes, err := kubeadmutil.ParseTemplate(CoreDNSConfigMap, struct{ DNSDomain, UpstreamNameserver, StubDomain string }{
+				DNSDomain:          "cluster.local",
+				UpstreamNameserver: "/etc/resolv.conf",
+				StubDomain:         "",
+			})
 			if err != nil {
-				t.Fatalf("unable to fetch CoreDNS current installed version and ConfigMap.")
+				t.Errorf("unexpected ParseTemplate failure: %+v", err)
 			}
-			err = migrateCoreDNSCorefile(client, cm, corefile, currentInstalledCoreDNSVersion)
+
+			err = createCoreDNSAddon(nil, nil, configMapBytes, client)
 			if err != nil {
-				t.Fatalf("error creating the CoreDNS ConfigMap: %v", err)
+				t.Fatalf("error creating the CoreDNS Addon: %v", err)
 			}
 			migratedConfigMap, _ := client.CoreV1().ConfigMaps(metav1.NamespaceSystem).Get(context.TODO(), kubeadmconstants.CoreDNSConfigMap, metav1.GetOptions{})
 			if !strings.EqualFold(migratedConfigMap.Data["Corefile"], tc.expectedCorefileData) {