diff --git a/hypershift-operator/controllers/nodepool/capi.go b/hypershift-operator/controllers/nodepool/capi.go index 1a91de160a..9386d54e5f 100644 --- a/hypershift-operator/controllers/nodepool/capi.go +++ b/hypershift-operator/controllers/nodepool/capi.go @@ -1056,65 +1056,6 @@ func (c *CAPI) reconcileMachineSet(ctx context.Context, } } - // Bubble up upgrading NodePoolUpdatingVersionConditionType. - var status corev1.ConditionStatus - reason := "" - message := "" - status = "unknown" - removeStatusCondition(&nodePool.Status.Conditions, hyperv1.NodePoolUpdatingVersionConditionType) - - if _, ok := machineSet.Annotations[nodePoolAnnotationUpgradeInProgressTrue]; ok { - status = corev1.ConditionTrue - reason = hyperv1.AsExpectedReason - } - - if _, ok := machineSet.Annotations[nodePoolAnnotationUpgradeInProgressFalse]; ok { - status = corev1.ConditionFalse - reason = hyperv1.NodePoolInplaceUpgradeFailedReason - } - - // Check if config needs to be updated. - isUpdatingConfig := isUpdatingConfig(nodePool, targetConfigHash) - - // Check if version needs to be updated. - isUpdatingVersion := isUpdatingVersion(nodePool, targetVersion) - - if isUpdatingVersion { - message = fmt.Sprintf("Updating Version, Target: %v", machineSet.Annotations[nodePoolAnnotationTargetConfigVersion]) - SetStatusCondition(&nodePool.Status.Conditions, hyperv1.NodePoolCondition{ - Type: hyperv1.NodePoolUpdatingVersionConditionType, - Status: status, - ObservedGeneration: nodePool.Generation, - Message: message, - Reason: reason, - }) - } else { - SetStatusCondition(&nodePool.Status.Conditions, hyperv1.NodePoolCondition{ - Type: hyperv1.NodePoolUpdatingVersionConditionType, - Status: corev1.ConditionFalse, - ObservedGeneration: nodePool.Generation, - Reason: hyperv1.AsExpectedReason, - }) - } - - if isUpdatingConfig { - message = fmt.Sprintf("Updating Config, Target: %v", machineSet.Annotations[nodePoolAnnotationTargetConfigVersion]) - SetStatusCondition(&nodePool.Status.Conditions, hyperv1.NodePoolCondition{ - Type: hyperv1.NodePoolUpdatingConfigConditionType, - Status: status, - ObservedGeneration: nodePool.Generation, - Message: message, - Reason: reason, - }) - } else { - SetStatusCondition(&nodePool.Status.Conditions, hyperv1.NodePoolCondition{ - Type: hyperv1.NodePoolUpdatingConfigConditionType, - Status: corev1.ConditionFalse, - ObservedGeneration: nodePool.Generation, - Reason: hyperv1.AsExpectedReason, - }) - } - // Bubble up AvailableReplicas and Ready condition from MachineSet. nodePool.Status.Replicas = machineSet.Status.AvailableReplicas for _, c := range machineSet.Status.Conditions { diff --git a/hypershift-operator/controllers/nodepool/conditions.go b/hypershift-operator/controllers/nodepool/conditions.go index 57c7414335..5b482fef93 100644 --- a/hypershift-operator/controllers/nodepool/conditions.go +++ b/hypershift-operator/controllers/nodepool/conditions.go @@ -347,11 +347,42 @@ func (r *NodePoolReconciler) updatingConfigCondition(ctx context.Context, nodePo targetConfigHash := token.HashWithoutVersion() isUpdatingConfig := isUpdatingConfig(nodePool, targetConfigHash) if isUpdatingConfig { + reason := hyperv1.AsExpectedReason + message := fmt.Sprintf("Updating config in progress. Target config: %s", targetConfigHash) + status := corev1.ConditionTrue + + if nodePool.Spec.Management.UpgradeType == hyperv1.UpgradeTypeInPlace { + capi, err := newCAPI(token, hcluster.Spec.InfraID) + if err != nil { + return &ctrl.Result{}, fmt.Errorf("error getting capi client: %w", err) + } + + machineSet := capi.machineSet() + err = r.Get(ctx, client.ObjectKeyFromObject(machineSet), machineSet) + if err != nil { + if !apierrors.IsNotFound(err) { + return &ctrl.Result{}, fmt.Errorf("failed to get MachineSet: %w", err) + } + } else { + if _, ok := machineSet.Annotations[nodePoolAnnotationUpgradeInProgressTrue]; ok { + status = corev1.ConditionTrue + reason = hyperv1.AsExpectedReason + message = machineSet.Annotations[nodePoolAnnotationUpgradeInProgressTrue] + } + + if _, ok := machineSet.Annotations[nodePoolAnnotationUpgradeInProgressFalse]; ok { + status = corev1.ConditionFalse + reason = hyperv1.NodePoolInplaceUpgradeFailedReason + message = machineSet.Annotations[nodePoolAnnotationUpgradeInProgressFalse] + } + } + } + SetStatusCondition(&nodePool.Status.Conditions, hyperv1.NodePoolCondition{ Type: hyperv1.NodePoolUpdatingConfigConditionType, - Status: corev1.ConditionTrue, - Reason: hyperv1.AsExpectedReason, - Message: fmt.Sprintf("Updating config in progress. Target config: %s", targetConfigHash), + Status: status, + Reason: reason, + Message: message, ObservedGeneration: nodePool.Generation, }) log.Info("NodePool config is updating", @@ -378,11 +409,47 @@ func (r *NodePoolReconciler) updatingVersionCondition(ctx context.Context, nodeP targetVersion := releaseImage.Version() isUpdatingVersion := isUpdatingVersion(nodePool, targetVersion) if isUpdatingVersion { + reason := hyperv1.AsExpectedReason + message := fmt.Sprintf("Updating version in progress. Target version: %s", targetVersion) + status := corev1.ConditionTrue + + if nodePool.Spec.Management.UpgradeType == hyperv1.UpgradeTypeInPlace { + token, err := r.token(ctx, hcluster, nodePool) + if err != nil { + return &ctrl.Result{}, fmt.Errorf("error getting token: %w", err) + } + + capi, err := newCAPI(token, hcluster.Spec.InfraID) + if err != nil { + return &ctrl.Result{}, fmt.Errorf("error getting capi client: %w", err) + } + + machineSet := capi.machineSet() + err = r.Get(ctx, client.ObjectKeyFromObject(machineSet), machineSet) + if err != nil { + if !apierrors.IsNotFound(err) { + return &ctrl.Result{}, fmt.Errorf("failed to get MachineSet: %w", err) + } + } else { + if _, ok := machineSet.Annotations[nodePoolAnnotationUpgradeInProgressTrue]; ok { + status = corev1.ConditionTrue + reason = hyperv1.AsExpectedReason + message = machineSet.Annotations[nodePoolAnnotationUpgradeInProgressTrue] + } + + if _, ok := machineSet.Annotations[nodePoolAnnotationUpgradeInProgressFalse]; ok { + status = corev1.ConditionFalse + reason = hyperv1.NodePoolInplaceUpgradeFailedReason + message = machineSet.Annotations[nodePoolAnnotationUpgradeInProgressFalse] + } + } + } + SetStatusCondition(&nodePool.Status.Conditions, hyperv1.NodePoolCondition{ Type: hyperv1.NodePoolUpdatingVersionConditionType, - Status: corev1.ConditionTrue, - Reason: hyperv1.AsExpectedReason, - Message: fmt.Sprintf("Updating version in progress. Target version: %s", targetVersion), + Status: status, + Reason: reason, + Message: message, ObservedGeneration: nodePool.Generation, }) log.Info("NodePool version is updating", diff --git a/hypershift-operator/controllers/nodepool/conditions_test.go b/hypershift-operator/controllers/nodepool/conditions_test.go index 2d03bb9c8b..6b7d3fee23 100644 --- a/hypershift-operator/controllers/nodepool/conditions_test.go +++ b/hypershift-operator/controllers/nodepool/conditions_test.go @@ -1,7 +1,18 @@ package nodepool import ( + "context" "fmt" + "github.com/blang/semver" + "github.com/openshift/api/image/docker10" + "github.com/openshift/hypershift/hypershift-operator/controllers/manifests/ignitionserver" + "github.com/openshift/hypershift/support/api" + fakereleaseprovider "github.com/openshift/hypershift/support/releaseinfo/fake" + "github.com/openshift/hypershift/support/thirdparty/library-go/pkg/image/dockerv1client" + "github.com/openshift/hypershift/support/util/fakeimagemetadataprovider" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/cluster-api/api/v1beta1" + "sigs.k8s.io/controller-runtime/pkg/client/fake" "testing" "time" @@ -85,3 +96,401 @@ func TestGenerateReconciliationPausedCondition(t *testing.T) { }) } } + +func TestUpdatingConfigCondition(t *testing.T) { + g := NewGomegaWithT(t) + + tests := []struct { + name string + upgradeType hyperv1.UpgradeType + machineSetExists bool + machineSetUpgradeFail bool + isUpdatingConfig bool + expectedStatus corev1.ConditionStatus + expectedReason string + expectedMessagePart string + }{ + { + name: "NodePool is Replace and not updating config", + expectedStatus: corev1.ConditionFalse, + expectedReason: hyperv1.AsExpectedReason, + expectedMessagePart: "", + machineSetExists: true, + isUpdatingConfig: false, + }, + { + name: "NodePool is Replace and updating config", + upgradeType: hyperv1.UpgradeTypeReplace, + expectedStatus: corev1.ConditionTrue, + expectedReason: hyperv1.AsExpectedReason, + expectedMessagePart: "Updating config in progress. Target config:", + machineSetExists: true, + isUpdatingConfig: true, + }, + { + name: "NodePool is InPlace, machineSet does not exist and updating config", + upgradeType: hyperv1.UpgradeTypeInPlace, + machineSetExists: false, + isUpdatingConfig: true, + expectedStatus: corev1.ConditionTrue, + expectedReason: hyperv1.AsExpectedReason, + expectedMessagePart: "Updating config in progress. Target config:", + }, + { + name: "NodePool is InPlace, machineSet exists, and updating config", + upgradeType: hyperv1.UpgradeTypeInPlace, + machineSetExists: true, + machineSetUpgradeFail: false, + isUpdatingConfig: true, + expectedStatus: corev1.ConditionTrue, + expectedReason: hyperv1.AsExpectedReason, + expectedMessagePart: "true", + }, + { + name: "NodePool is InPlace, machineSet exists, and updating config fails", + upgradeType: hyperv1.UpgradeTypeInPlace, + machineSetExists: true, + machineSetUpgradeFail: true, + isUpdatingConfig: true, + expectedStatus: corev1.ConditionFalse, + expectedReason: hyperv1.NodePoolInplaceUpgradeFailedReason, + expectedMessagePart: "true", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + ctx := context.TODO() + + pullSecret, ignitionServerCACert, machineConfig, ignitionConfig, ignitionConfig2, ignitionConfig3 := setupTestObjects() + + hostedCluster := &hyperv1.HostedCluster{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster-name", Namespace: "myns"}, + Spec: hyperv1.HostedClusterSpec{ + PullSecret: corev1.LocalObjectReference{Name: pullSecret.Name}, + InfraID: "fake-infra-id", + }, + Status: hyperv1.HostedClusterStatus{ + IgnitionEndpoint: "https://ignition.cluster-name.myns.devcluster.openshift.com", + }, + } + + //change pull secret name to simulate node pool config update + if tc.isUpdatingConfig { + hostedCluster.Spec.PullSecret.Name = "new-pull" + pullSecret.ObjectMeta.Name = "new-pull" + } + + nodePool := &hyperv1.NodePool{ + ObjectMeta: metav1.ObjectMeta{ + Name: "nodepool-name", + Namespace: "myns", + Annotations: map[string]string{ + nodePoolAnnotationCurrentConfig: "08e4f890", + }, + }, + Spec: hyperv1.NodePoolSpec{ + ClusterName: hostedCluster.Name, + Release: hyperv1.Release{ + Image: "fake-release-image"}, + Management: hyperv1.NodePoolManagement{ + UpgradeType: tc.upgradeType, + }, + Config: []corev1.LocalObjectReference{ + {Name: "machineconfig-1"}, + }, + }, + Status: hyperv1.NodePoolStatus{ + Version: semver.MustParse("4.18.0").String(), + }, + } + + client := fake.NewClientBuilder().WithScheme(api.Scheme).WithObjects( + nodePool, + hostedCluster, + pullSecret, + ignitionServerCACert, + machineConfig, + ignitionConfig, + ignitionConfig2, + ignitionConfig3, + ).Build() + + r := &NodePoolReconciler{ + Client: client, + ReleaseProvider: &fakereleaseprovider.FakeReleaseProvider{Version: semver.MustParse("4.18.0").String()}, + ImageMetadataProvider: &fakeimagemetadataprovider.FakeImageMetadataProvider{Result: &dockerv1client.DockerImageConfig{Config: &docker10.DockerConfig{ + Labels: map[string]string{}, + }}}, + } + + if tc.machineSetExists { + machineSet := setUpDummyMachineSet(nodePool, hostedCluster, tc.machineSetUpgradeFail) + + err := client.Create(ctx, machineSet) + if err != nil { + return + } + } + + _, err := r.updatingConfigCondition(ctx, nodePool, hostedCluster) + g.Expect(err).To(BeNil()) + + condition := FindStatusCondition(nodePool.Status.Conditions, hyperv1.NodePoolUpdatingConfigConditionType) + g.Expect(condition).NotTo(BeNil()) + g.Expect(condition.Status).To(Equal(tc.expectedStatus)) + g.Expect(condition.Reason).To(Equal(tc.expectedReason)) + if tc.expectedMessagePart != "" { + g.Expect(condition.Message).To(ContainSubstring(tc.expectedMessagePart)) + } + }) + } +} + +func TestUpdatingVersionCondition(t *testing.T) { + g := NewGomegaWithT(t) + + tests := []struct { + name string + upgradeType hyperv1.UpgradeType + machineSetExists bool + machineSetUpgradeFail bool + isUpdatingVersion bool + expectedStatus corev1.ConditionStatus + expectedReason string + expectedMessagePart string + }{ + { + name: "NodePool is Replace and not updating version", + expectedStatus: corev1.ConditionFalse, + expectedReason: hyperv1.AsExpectedReason, + expectedMessagePart: "", + machineSetExists: true, + isUpdatingVersion: false, + }, + { + name: "NodePool is Replace and updating version", + upgradeType: hyperv1.UpgradeTypeReplace, + expectedStatus: corev1.ConditionTrue, + expectedReason: hyperv1.AsExpectedReason, + expectedMessagePart: "Updating version in progress. Target version:", + machineSetExists: true, + isUpdatingVersion: true, + }, + { + name: "NodePool is InPlace, machineSet does not exist and updating version", + upgradeType: hyperv1.UpgradeTypeInPlace, + machineSetExists: false, + isUpdatingVersion: true, + expectedStatus: corev1.ConditionTrue, + expectedReason: hyperv1.AsExpectedReason, + expectedMessagePart: "Updating version in progress. Target version:", + }, + { + name: "NodePool is InPlace, machineSet exists, and updating version", + upgradeType: hyperv1.UpgradeTypeInPlace, + machineSetExists: true, + machineSetUpgradeFail: false, + isUpdatingVersion: true, + expectedStatus: corev1.ConditionTrue, + expectedReason: hyperv1.AsExpectedReason, + expectedMessagePart: "true", + }, + { + name: "NodePool is InPlace, machineSet exists, and updating version fails", + upgradeType: hyperv1.UpgradeTypeInPlace, + machineSetExists: true, + machineSetUpgradeFail: true, + isUpdatingVersion: true, + expectedStatus: corev1.ConditionFalse, + expectedReason: hyperv1.NodePoolInplaceUpgradeFailedReason, + expectedMessagePart: "true", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + ctx := context.TODO() + + pullSecret, ignitionServerCACert, machineConfig, ignitionConfig, ignitionConfig2, ignitionConfig3 := setupTestObjects() + + hostedCluster := &hyperv1.HostedCluster{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster-name", Namespace: "myns"}, + Spec: hyperv1.HostedClusterSpec{ + PullSecret: corev1.LocalObjectReference{Name: pullSecret.Name}, + InfraID: "fake-infra-id", + }, + Status: hyperv1.HostedClusterStatus{ + IgnitionEndpoint: "https://ignition.cluster-name.myns.devcluster.openshift.com", + }, + } + + nodePool := &hyperv1.NodePool{ + ObjectMeta: metav1.ObjectMeta{ + Name: "nodepool-name", + Namespace: "myns", + }, + Spec: hyperv1.NodePoolSpec{ + ClusterName: hostedCluster.Name, + Release: hyperv1.Release{ + Image: "fake-release-image"}, + Management: hyperv1.NodePoolManagement{ + UpgradeType: tc.upgradeType, + }, + Config: []corev1.LocalObjectReference{ + {Name: "machineconfig-1"}, + }, + }, + Status: hyperv1.NodePoolStatus{ + Version: semver.MustParse("4.18.0").String(), + }, + } + + if tc.isUpdatingVersion { + nodePool.Spec.Release.Image = "new-release-image" + nodePool.Status.Version = semver.MustParse("4.18.1").String() + } + + client := fake.NewClientBuilder().WithScheme(api.Scheme).WithObjects( + nodePool, + hostedCluster, + pullSecret, + ignitionServerCACert, + machineConfig, + ignitionConfig, + ignitionConfig2, + ignitionConfig3, + ).Build() + + r := &NodePoolReconciler{ + Client: client, + ReleaseProvider: &fakereleaseprovider.FakeReleaseProvider{Version: semver.MustParse("4.18.0").String()}, + ImageMetadataProvider: &fakeimagemetadataprovider.FakeImageMetadataProvider{Result: &dockerv1client.DockerImageConfig{Config: &docker10.DockerConfig{ + Labels: map[string]string{}, + }}}, + } + + if tc.machineSetExists { + machineSet := setUpDummyMachineSet(nodePool, hostedCluster, tc.machineSetUpgradeFail) + + err := client.Create(ctx, machineSet) + if err != nil { + return + } + } + + _, err := r.updatingVersionCondition(ctx, nodePool, hostedCluster) + g.Expect(err).To(BeNil()) + + condition := FindStatusCondition(nodePool.Status.Conditions, hyperv1.NodePoolUpdatingVersionConditionType) + g.Expect(condition).NotTo(BeNil()) + g.Expect(condition.Status).To(Equal(tc.expectedStatus)) + g.Expect(condition.Reason).To(Equal(tc.expectedReason)) + if tc.expectedMessagePart != "" { + g.Expect(condition.Message).To(ContainSubstring(tc.expectedMessagePart)) + } + }) + } +} + +func setupTestObjects() (*corev1.Secret, *corev1.Secret, *corev1.ConfigMap, *corev1.ConfigMap, *corev1.ConfigMap, *corev1.ConfigMap) { + coreMachineConfig := ` +apiVersion: machineconfiguration.openshift.io/v1 +kind: MachineConfig +metadata: + labels: + machineconfiguration.openshift.io/role: master + name: config-1 +spec: + config: + ignition: + version: 3.2.0 + storage: + files: + - contents: + source: "[Service]\nType=oneshot\nExecStart=/usr/bin/echo Hello World\n\n[Install]\nWantedBy=multi-user.target" + filesystem: root + mode: 493 + path: /usr/local/bin/file1.sh +` + + machineConfig := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "machineconfig-1", + Namespace: "myns", + }, + Data: map[string]string{ + TokenSecretConfigKey: coreMachineConfig, + }, + } + + ignitionConfig := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "core-machineconfig", + Namespace: "myns-cluster-name", + Labels: map[string]string{ + nodePoolCoreIgnitionConfigLabel: "true", + }, + }, + Data: map[string]string{ + TokenSecretConfigKey: coreMachineConfig, + }, + } + ignitionConfig2 := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "core-machineconfig-2", + Namespace: "myns-cluster-name", + Labels: map[string]string{ + nodePoolCoreIgnitionConfigLabel: "true", + }, + }, + Data: map[string]string{ + TokenSecretConfigKey: coreMachineConfig, + }, + } + ignitionConfig3 := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "core-machineconfig-3", + Namespace: "myns-cluster-name", + Labels: map[string]string{ + nodePoolCoreIgnitionConfigLabel: "true", + }, + }, + Data: map[string]string{ + TokenSecretConfigKey: coreMachineConfig, + }, + } + + ignitionServerCACert := ignitionserver.IgnitionCACertSecret("myns-cluster-name") + ignitionServerCACert.Data = map[string][]byte{ + corev1.TLSCertKey: []byte("test-ignition-ca-cert"), + } + + pullSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "pull-secret", Namespace: "myns"}, + Data: map[string][]byte{ + corev1.DockerConfigJsonKey: []byte("whatever"), + }, + } + + return pullSecret, ignitionServerCACert, machineConfig, ignitionConfig, ignitionConfig2, ignitionConfig3 +} + +func setUpDummyMachineSet(nodePool *hyperv1.NodePool, hostedCluster *hyperv1.HostedCluster, machineSetUpgradeFail bool) *v1beta1.MachineSet { + machineSet := &v1beta1.MachineSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: nodePool.Name, + Namespace: hostedCluster.Namespace + "-" + hostedCluster.Name, + Annotations: map[string]string{ + nodePoolAnnotationUpgradeInProgressTrue: "true", + }, + }, + } + + if machineSetUpgradeFail { + machineSet.Annotations = map[string]string{ + nodePoolAnnotationUpgradeInProgressFalse: "true", + } + } + return machineSet +}