Add a generic failure status, defer restoring logins on failure states

cleanup/ad-guid-unmigration.sh

@@ -239,7 +239,7 @@ until kubectl --namespace=cattle-system logs $pod_id -f
 do
     if [ $count -gt $job_start_timeout ]
     then
-        echo "Timout reached, check the job by running kubectl --namespace=cattle-system get jobs"
+        echo "Timeout reached, check the job by running kubectl --namespace=cattle-system get jobs"
         echo "To cleanup manually, you can run:"
         echo "  kubectl --namespace=cattle-system delete serviceaccount,job -l rancher-cleanup=true"
         echo "  kubectl delete clusterrole,clusterrolebinding -l rancher-cleanup=true"

pkg/agent/clean/adunmigration/migrate.go

@@ -148,14 +148,24 @@ func UnmigrateAdGUIDUsers(clientConfig *restclient.Config, dryRun bool, deleteMi
 		}
 	}
 
+	finalStatus := activedirectory.StatusMigrationFinished
+
 	// set the status to running and reset the unmigrated fields
 	if !dryRun {
 		err = updateMigrationStatus(sc, activedirectory.StatusMigrationField, activedirectory.StatusMigrationRunning)
-		updateUnmigratedUsers("", migrateStatusSkipped, true, sc)
-		updateUnmigratedUsers("", migrateStatusMissing, true, sc)
 		if err != nil {
 			return fmt.Errorf("unable to update migration status configmap: %v", err)
 		}
+		updateUnmigratedUsers("", migrateStatusSkipped, true, sc)
+		updateUnmigratedUsers("", migrateStatusMissing, true, sc)
+		// If we return past this point, no matter how we got there, make sure we update the configmap to clear the
+		// status away from "running." If we fail to do this, we block AD-based logins indefinitely.
+		defer func(sc *config.ScaledContext, status string, value string) {
+			err := updateMigrationStatus(sc, status, value)
+			if err != nil {
+				logrus.Errorf("[%v] unable to update migration status configmap: %v", migrateAdUserOperation, err)
+			}
+		}(sc, activedirectory.StatusMigrationField, finalStatus)
 	}
 
 	users, err := sc.Management.Users("").List(metav1.ListOptions{})
@@ -164,29 +174,44 @@ func UnmigrateAdGUIDUsers(clientConfig *restclient.Config, dryRun bool, deleteMi
 	}
 
 	usersToMigrate, missingUsers, skippedUsers := identifyMigrationWorkUnits(users, adConfig)
+	// If any of the below functions fail, there is either a permissions problem or a more serious issue with the
+	// Rancher API. We should bail in this case and not attempt to process users.
 	err = collectTokens(&usersToMigrate, sc)
 	if err != nil {
+		finalStatus = activedirectory.StatusMigrationFailed
 		return err
 	}
 	err = collectCRTBs(&usersToMigrate, sc)
 	if err != nil {
+		finalStatus = activedirectory.StatusMigrationFailed
 		return err
 	}
 	err = collectPRTBs(&usersToMigrate, sc)
 	if err != nil {
+		finalStatus = activedirectory.StatusMigrationFailed
 		return err
 	}
 	err = collectGRBs(&usersToMigrate, sc)
 	if err != nil {
+		finalStatus = activedirectory.StatusMigrationFailed
 		return err
 	}
 	err = migrateAllowedUserPrincipals(&usersToMigrate, sc, dryRun)
 	if err != nil {
+		finalStatus = activedirectory.StatusMigrationFailed
 		return err
 	}
 
+	if len(missingUsers) > 0 {
+		finalStatus = activedirectory.StatusMigrationFinishedWithMissing
+	}
+	if len(skippedUsers) > 0 {
+		finalStatus = activedirectory.StatusMigrationFinishedWithSkipped
+	}
+
 	for _, user := range skippedUsers {
-		logrus.Errorf("[%v] unable to migrate user '%v' due to a connection failure; this user will be skipped", migrateAdUserOperation, user.originalUser.Name)
+		logrus.Errorf("[%v] unable to migrate user '%v' due to a connection failure; this user will be skipped",
+			migrateAdUserOperation, user.originalUser.Name)
 		if !dryRun {
 			updateUnmigratedUsers(user.originalUser.Name, migrateStatusSkipped, false, sc)
 		}
@@ -249,23 +274,6 @@ func UnmigrateAdGUIDUsers(clientConfig *restclient.Config, dryRun bool, deleteMi
 		}
 	}
 
-	if !dryRun {
-		// If we have skipped users, that status will be reported as the overall status
-		// since that state is potentially resolvable by re-running the utility
-		var status string
-		if len(skippedUsers) > 0 {
-			status = activedirectory.StatusMigrationFinishedWithSkipped
-		} else if len(missingUsers) > 0 {
-			status = activedirectory.StatusMigrationFinishedWithMissing
-		} else {
-			status = activedirectory.StatusMigrationFinished
-		}
-		err = updateMigrationStatus(sc, activedirectory.StatusMigrationField, status)
-		if err != nil {
-			return fmt.Errorf("unable to update migration status configmap: %v", err)
-		}
-	}
-
 	return nil
 }
 

pkg/auth/providers/activedirectory/activedirectory_provider.go

@@ -39,6 +39,7 @@ const (
 	StatusMigrationRunning             = "Running"
 	StatusMigrationFinishedWithSkipped = "FinishedWithSkipped"
 	StatusMigrationFinishedWithMissing = "FinishedWithMissing"
+	StatusMigrationFailed              = "Failed"
 	StatusLoginDisabled                = "login is disabled while migration is running"
 )