Merge pull request #618 from pvasant/fixupgradeissue

Fix upgrade issue
openshift · Mar 2, 2022 · 16edbee · 16edbee
2 parents c1e0cc3 + 8cee604
commit 16edbee
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 65 deletions.
diff --git a/cmd/upgrade/accountroles/cmd.go b/cmd/upgrade/accountroles/cmd.go
@@ -18,10 +18,11 @@ package accountroles
 
 import (
 	"fmt"
-
 	"os"
 	"strings"
+	"time"
 
+	"github.com/briandowns/spinner"
 	"github.com/spf13/cobra"
 
 	"github.com/openshift/rosa/pkg/aws"
@@ -36,6 +37,7 @@ import (
 var args struct {
 	prefix                      string
 	isInvokedFromClusterUpgrade bool
+	clusterID                   string
 }
 
 var Cmd = &cobra.Command{
@@ -61,6 +63,7 @@ func init() {
 		"User-defined prefix for all generated AWS resources",
 	)
 	Cmd.MarkFlagRequired("prefix")
+
 	confirm.AddFlag(flags)
 	interactive.AddFlag(flags)
 }
@@ -71,13 +74,15 @@ func run(cmd *cobra.Command, argv []string) error {
 
 	isInvokedFromClusterUpgrade := false
 	skipInteractive := false
-	if len(argv) == 2 && !cmd.Flag("prefix").Changed {
+	if len(argv) >= 2 && !cmd.Flag("prefix").Changed {
 		args.prefix = argv[0]
 		aws.SetModeKey(argv[1])
-
 		if argv[1] != "" {
 			skipInteractive = true
 		}
+		if len(argv) > 2 && argv[2] != "" {
+			args.clusterID = argv[2]
+		}
 		isInvokedFromClusterUpgrade = true
 	}
 	args.isInvokedFromClusterUpgrade = isInvokedFromClusterUpgrade
@@ -86,9 +91,7 @@ func run(cmd *cobra.Command, argv []string) error {
 		reporter.Errorf("%s", err)
 		os.Exit(1)
 	}
-
 	prefix := args.prefix
-
 	// Create the AWS client:
 	awsClient, err := aws.NewClient().
 		Logger(logger).
@@ -125,6 +128,17 @@ func run(cmd *cobra.Command, argv []string) error {
 		os.Exit(1)
 	}
 
+	var spin *spinner.Spinner
+	if reporter.IsTerminal() {
+		spin = spinner.New(spinner.CharSets[9], 100*time.Millisecond)
+	}
+	if spin != nil {
+		spin.Start()
+	}
+	if !args.isInvokedFromClusterUpgrade {
+		reporter.Infof("Ensuring account and operator role policies compatibility for upgrade")
+	}
+
 	isUpgradeNeedForAccountRolePolicies, err := awsClient.IsUpgradedNeededForAccountRolePolicies(prefix,
 		aws.DefaultPolicyVersion)
 	if err != nil {
@@ -135,12 +149,18 @@ func run(cmd *cobra.Command, argv []string) error {
 	isUpgradeNeedForOperatorRolePolicies, err := awsClient.IsUpgradedNeededForOperatorRolePoliciesUsingPrefix(prefix,
 		creator.AccountID,
 		aws.DefaultPolicyVersion)
+
 	if err != nil {
 		reporter.Errorf("%s", err)
 		os.Exit(1)
 	}
-
+	if spin != nil {
+		spin.Stop()
+	}
 	if !isUpgradeNeedForAccountRolePolicies && !isUpgradeNeedForOperatorRolePolicies {
+		if args.isInvokedFromClusterUpgrade {
+			return nil
+		}
 		reporter.Infof("Account role with the prefix '%s' is already up-to-date.", prefix)
 		os.Exit(0)
 	}
@@ -200,6 +220,13 @@ func run(cmd *cobra.Command, argv []string) error {
 		commands := buildCommands(prefix, creator.AccountID, isUpgradeNeedForAccountRolePolicies,
 			isUpgradeNeedForOperatorRolePolicies)
 		fmt.Println(commands)
+		if args.isInvokedFromClusterUpgrade {
+			reporter.Infof("Run the following command to continue scheduling cluster upgrade"+
+				" once account and operator roles have been upgraded : \n\n"+
+				"\trosa upgrade cluster --cluster %s\n", args.clusterID)
+			os.Exit(0)
+		}
+
 	default:
 		reporter.Errorf("Invalid mode. Allowed values are %s", aws.Modes)
 		os.Exit(1)
@@ -210,7 +237,7 @@ func run(cmd *cobra.Command, argv []string) error {
 func upgradeAccountRolePolicies(reporter *rprtr.Object, awsClient aws.Client, prefix string, accountID string) error {
 	for file, role := range aws.AccountRoles {
 		name := aws.GetRoleName(prefix, role.Name)
-		if !confirm.Prompt(true, "Upgrade the '%s' role polices to version %s?", name,
+		if !confirm.Prompt(true, "Upgrade the '%s' role policy to version %s?", name,
 			aws.DefaultPolicyVersion) {
 			if args.isInvokedFromClusterUpgrade {
 				return reporter.Errorf("Account roles need to be upgraded to proceed" +

diff --git a/cmd/upgrade/cluster/cmd.go b/cmd/upgrade/cluster/cmd.go
@@ -23,7 +23,6 @@ import (
 	"strings"
 	"time"
 
-	"github.com/briandowns/spinner"
 	cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
@@ -233,72 +232,25 @@ func run(cmd *cobra.Command, _ []string) {
 
 	// if cluster is sts validate roles are compatible with upgrade version
 	if isSTS {
-
-		var spin *spinner.Spinner
-		if reporter.IsTerminal() {
-			spin = spinner.New(spinner.CharSets[9], 100*time.Millisecond)
-		}
 		reporter.Infof("Ensuring account and operator role policies for cluster '%s'"+
 			" are compatible with upgrade.", cluster.ID())
-		if spin != nil {
-			spin.Start()
-		}
 		prefix, err := aws.GetPrefixFromAccountRole(cluster)
 		if err != nil {
 			reporter.Errorf("Could not get role prefix for cluster '%s' : %v", clusterKey, err)
 			os.Exit(1)
 		}
-
-		isAccountRoleUpgradeNeeded, err := awsClient.IsUpgradedNeededForAccountRolePolicies(prefix, version)
-		if err != nil {
-			reporter.Errorf("Could not validate '%s' clusters account roles : %v", clusterKey, err)
-			os.Exit(1)
-		}
-
-		isOperatorRoleUpgradeNeeded, err := awsClient.IsUpgradedNeededForOperatorRolePolicies(cluster,
-			awsCreator.AccountID, version)
+		err = accountroles.Cmd.RunE(accountroles.Cmd, []string{prefix, mode, cluster.ID()})
 		if err != nil {
-			reporter.Errorf("Could not validate '%s' clusters operator roles : %v", clusterKey, err)
-			os.Exit(1)
-		}
-		if spin != nil {
-			spin.Stop()
-		}
-		if isAccountRoleUpgradeNeeded || isOperatorRoleUpgradeNeeded {
-			reporter.Infof("Account and/or operator roles needed upgrade")
-			if interactive.Enabled() || mode == "" {
-				mode, err = interactive.GetOption(interactive.Input{
-					Question: "Upgrade mode",
-					Help:     cmd.Flags().Lookup("mode").Usage,
-					Default:  aws.ModeAuto,
-					Options:  aws.Modes,
-					Required: true,
-				})
-				if err != nil {
-					reporter.Errorf("Expected a valid cluster upgrade mode: %s", err)
-					os.Exit(1)
-				}
-			}
-			err := accountroles.Cmd.RunE(accountroles.Cmd, []string{prefix, mode})
-			if err != nil {
-				accountRoleStr := fmt.Sprintf("rosa upgrade account-roles --prefix %s", prefix)
-				operatorRoleStr := fmt.Sprintf("rosa upgrade operator-roles -c %s", clusterKey)
-
-				reporter.Infof("Account and/or Operator Role policies are not valid with upgrade version %s. "+
-					"Run the following command(s) to upgrade the roles:\n\n"+
-					"\t%s\n"+
-					"\t%s\n", version, accountRoleStr, operatorRoleStr)
-				mode = aws.ModeManual
-			}
-			if mode == aws.ModeManual {
-				reporter.Infof("Run the following command to continue scheduling cluster upgrade"+
-					" once account and operator roles have been upgraded : \n\n"+
-					"\trosa upgrade cluster --cluster %s\n", clusterKey)
-				os.Exit(0)
-			}
-		} else {
-			reporter.Infof("Account and operator roles for cluster '%s' are compatible with upgrade", clusterKey)
+			accountRoleStr := fmt.Sprintf("rosa upgrade account-roles --prefix %s", prefix)
+			operatorRoleStr := fmt.Sprintf("rosa upgrade operator-roles -c %s", clusterKey)
+
+			reporter.Infof("Account and/or Operator Role policies are not valid with upgrade version %s. "+
+				"Run the following command(s) to upgrade the roles:\n\n"+
+				"\t%s\n"+
+				"\t%s\n", version, accountRoleStr, operatorRoleStr)
+			os.Exit(0)
 		}
+		reporter.Infof("Account and operator roles for cluster '%s' are compatible with upgrade", clusterKey)
 	}
 
 	upgradePolicyBuilder := cmv1.NewUpgradePolicy().

diff --git a/pkg/aws/policies.go b/pkg/aws/policies.go
@@ -1482,6 +1482,9 @@ func (c *awsClient) validateRoleUpgradeVersionCompatibility(roleName string,
 		return false, err
 	}
 	for _, attachedPolicy := range attachedPolicies {
+		if attachedPolicy.PolicyArn == "" {
+			continue
+		}
 		isCompatible, err := c.isRolePoliciesCompatibleForUpgrade(attachedPolicy.PolicyArn, version)
 		if err != nil {
 			return false, errors.Errorf("Failed to validate role polices : %v", err)