diff --git a/pkg/cli/admin/upgrade/status/status.go b/pkg/cli/admin/upgrade/status/status.go index 26f5be1be5..8245accb93 100644 --- a/pkg/cli/admin/upgrade/status/status.go +++ b/pkg/cli/admin/upgrade/status/status.go @@ -55,7 +55,9 @@ func New(f kcmdutil.Factory, streams genericiooptions.IOStreams) *cobra.Command Short: "Display the status of the current cluster version update or multi-arch migration", Run: func(cmd *cobra.Command, args []string) { kcmdutil.CheckErr(o.Complete(f, cmd, args)) - kcmdutil.CheckErr(o.Run(cmd.Context())) + ctx, cancel := context.WithTimeout(cmd.Context(), 5*time.Minute) + defer cancel() + kcmdutil.CheckErr(o.Run(ctx)) }, } @@ -109,6 +111,7 @@ func (o *options) Complete(f kcmdutil.Factory, cmd *cobra.Command, args []string return err } cfg.UserAgent = rest.DefaultKubernetesUserAgent() + "(upgrade-status)" + cfg.Timeout = 60 * time.Second roundTripper, err := rest.TransportFor(cfg) if err != nil { @@ -230,23 +233,13 @@ func (o *options) Run(ctx context.Context) error { } var machineConfigs *machineconfigv1.MachineConfigList if machineConfigs = o.mockData.machineConfigs; machineConfigs == nil { - machineConfigs = &machineconfigv1.MachineConfigList{} - for _, node := range allNodes.Items { - for _, key := range []string{mco.CurrentMachineConfigAnnotationKey, mco.DesiredMachineConfigAnnotationKey} { - machineConfigName, ok := node.Annotations[key] - if !ok || machineConfigName == "" { - continue - } - mc, err := getMachineConfig(ctx, o.MachineConfigClient, machineConfigs.Items, machineConfigName) - if err != nil { - return err - } - if mc != nil { - machineConfigs.Items = append(machineConfigs.Items, *mc) - } - } + var err error + machineConfigs, err = o.MachineConfigClient.MachineconfigurationV1().MachineConfigs().List(ctx, metav1.ListOptions{}) + if err != nil { + return fmt.Errorf("listing MachineConfigs: %w", err) } } + machineConfigVersionMap := buildMachineConfigVersionMap(machineConfigs.Items) // The cluster stays within the same version number when it is migrated to multi-arch // and thus the comparison of version numbers in a normal cluster upgrade does not work. @@ -298,7 +291,7 @@ func (o *options) Run(ctx context.Context) error { // but it cannot be opted out at the moment. multiArchMigrationInProgress := multiArchMigration(cv.Status.History) && progressing.Status == configv1.ConditionTrue for _, pool := range pools.Items { - nodesStatusData, insights := assessNodesStatus(cv, pool, nodesPerPool[pool.Name], machineConfigs.Items, multiArchMigrationInProgress) + nodesStatusData, insights := assessNodesStatus(cv, pool, nodesPerPool[pool.Name], machineConfigVersionMap, multiArchMigrationInProgress) updateInsights = append(updateInsights, insights...) poolStatus, insights := assessMachineConfigPool(pool, nodesStatusData) updateInsights = append(updateInsights, insights...) diff --git a/pkg/cli/admin/upgrade/status/workerpool.go b/pkg/cli/admin/upgrade/status/workerpool.go index 2bd146940b..6f0d079d70 100644 --- a/pkg/cli/admin/upgrade/status/workerpool.go +++ b/pkg/cli/admin/upgrade/status/workerpool.go @@ -1,7 +1,6 @@ package status import ( - "context" "fmt" "io" "sort" @@ -10,12 +9,10 @@ import ( "time" corev1 "k8s.io/api/core/v1" - v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" configv1 "github.com/openshift/api/config/v1" mcfgv1 "github.com/openshift/api/machineconfiguration/v1" - mcfgv1client "github.com/openshift/client-go/machineconfiguration/clientset/versioned" "github.com/openshift/oc/pkg/cli/admin/upgrade/status/mco" ) @@ -114,15 +111,6 @@ type poolDisplayData struct { Nodes []nodeDisplayData } -func getMachineConfig(ctx context.Context, client mcfgv1client.Interface, machineConfigs []mcfgv1.MachineConfig, machineConfigName string) (*mcfgv1.MachineConfig, error) { - for _, mc := range machineConfigs { - if mc.Name == machineConfigName { - return nil, nil - } - } - return client.MachineconfigurationV1().MachineConfigs().Get(ctx, machineConfigName, v1.GetOptions{}) -} - func whichPool(master, worker labels.Selector, custom map[string]labels.Selector, node corev1.Node) string { if master.Matches(labels.Set(node.Labels)) { return "master" @@ -146,13 +134,13 @@ func ellipsizeNames(message string, name string) string { return strings.Replace(message, name, "", -1) } -func assessNodesStatus(cv *configv1.ClusterVersion, pool mcfgv1.MachineConfigPool, nodes []corev1.Node, machineConfigs []mcfgv1.MachineConfig, multiArchMigrationInProgress bool) ([]nodeDisplayData, []updateInsight) { +func assessNodesStatus(cv *configv1.ClusterVersion, pool mcfgv1.MachineConfigPool, nodes []corev1.Node, machineConfigVersionMap map[string]string, multiArchMigrationInProgress bool) ([]nodeDisplayData, []updateInsight) { var nodesStatusData []nodeDisplayData var insights []updateInsight for _, node := range nodes { desiredConfig, ok := node.Annotations[mco.DesiredMachineConfigAnnotationKey] - currentVersion, foundCurrent := getOpenShiftVersionOfMachineConfig(machineConfigs, node.Annotations[mco.CurrentMachineConfigAnnotationKey]) - desiredVersion, foundDesired := getOpenShiftVersionOfMachineConfig(machineConfigs, desiredConfig) + currentVersion, foundCurrent := getOpenShiftVersionOfMachineConfig(machineConfigVersionMap, node.Annotations[mco.CurrentMachineConfigAnnotationKey]) + desiredVersion, foundDesired := getOpenShiftVersionOfMachineConfig(machineConfigVersionMap, desiredConfig) lns := mco.NewLayeredNodeState(&node) isUnavailable := lns.IsUnavailable(&pool) @@ -245,14 +233,19 @@ func assessNodesStatus(cv *configv1.ClusterVersion, pool mcfgv1.MachineConfigPoo return nodesStatusData, insights } -func getOpenShiftVersionOfMachineConfig(machineConfigs []mcfgv1.MachineConfig, name string) (string, bool) { +func buildMachineConfigVersionMap(machineConfigs []mcfgv1.MachineConfig) map[string]string { + versionMap := make(map[string]string, len(machineConfigs)) for _, mc := range machineConfigs { - if mc.Name == name { - openshiftVersion := mc.Annotations[mco.ReleaseImageVersionAnnotationKey] - return openshiftVersion, openshiftVersion != "" + if version := mc.Annotations[mco.ReleaseImageVersionAnnotationKey]; version != "" { + versionMap[mc.Name] = version } } - return "", false + return versionMap +} + +func getOpenShiftVersionOfMachineConfig(versionMap map[string]string, name string) (string, bool) { + version, found := versionMap[name] + return version, found } func isNodeDraining(node corev1.Node, isUpdating bool) bool { diff --git a/pkg/cli/admin/upgrade/status/workerpool_test.go b/pkg/cli/admin/upgrade/status/workerpool_test.go index f66600c4b4..8f6d53e7a7 100644 --- a/pkg/cli/admin/upgrade/status/workerpool_test.go +++ b/pkg/cli/admin/upgrade/status/workerpool_test.go @@ -836,7 +836,8 @@ func Test_assessNodesStatus(t *testing.T) { } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - nodeDisplayData, updateInsight := assessNodesStatus(tc.args.cv, tc.args.pool, tc.args.nodes, tc.args.machineConfigs, false) + machineConfigVersionMap := buildMachineConfigVersionMap(tc.args.machineConfigs) + nodeDisplayData, updateInsight := assessNodesStatus(tc.args.cv, tc.args.pool, tc.args.nodes, machineConfigVersionMap, false) if diff := cmp.Diff(tc.expectedNodeDisplayData, nodeDisplayData, allowUnexportedWorkerPools); diff != "" { t.Errorf("nodeDisplayData differ from expected:\n%s", diff) } @@ -951,7 +952,8 @@ func Test_assessNodesStatus_DisplayData_Sorting(t *testing.T) { } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - nodeDisplayData, _ := assessNodesStatus(tc.args.cv, tc.args.pool, tc.args.nodes, tc.args.machineConfigs, false) + machineConfigVersionMap := buildMachineConfigVersionMap(tc.args.machineConfigs) + nodeDisplayData, _ := assessNodesStatus(tc.args.cv, tc.args.pool, tc.args.nodes, machineConfigVersionMap, false) if diff := cmp.Diff(tc.expectedNodeDisplayData, nodeDisplayData, allowUnexportedWorkerPools); diff != "" { t.Errorf("nodeDisplayData differ from expected:\n%s", diff) }