From 5421ebf06a8c120f6fcb06f6563586f88f237b31 Mon Sep 17 00:00:00 2001 From: IONOS Cloud SDKs & Tooling <73121773+ionoscloudsdk@users.noreply.github.com> Date: Tue, 3 Sep 2024 11:51:15 +0300 Subject: [PATCH] fix: ssset add hostname to status (#261) --- apis/compute/v1alpha1/serverset_types.go | 1 + .../serverset/recreate_only_bootvolume.go | 1 + internal/controller/serverset/serverset.go | 54 ++++++++++++------- .../controller/serverset/serverset_test.go | 25 ++++++--- ...e.ionoscloud.crossplane.io_serversets.yaml | 3 ++ ...loud.crossplane.io_statefulserversets.yaml | 3 ++ 6 files changed, 61 insertions(+), 26 deletions(-) diff --git a/apis/compute/v1alpha1/serverset_types.go b/apis/compute/v1alpha1/serverset_types.go index 814cd953..39f74c8a 100644 --- a/apis/compute/v1alpha1/serverset_types.go +++ b/apis/compute/v1alpha1/serverset_types.go @@ -137,6 +137,7 @@ type ServerSetReplicaStatus struct { // +kubebuilder:validation:Enum=ACTIVE;PASSIVE Role Role `json:"role"` Name string `json:"name"` + Hostname string `json:"hostname"` ReplicaIndex int `json:"replicaIndex"` NICStatuses []NicStatus `json:"nicStatus,omitempty"` // +kubebuilder:validation:Enum=UNKNOWN;READY;ERROR;BUSY diff --git a/internal/controller/serverset/recreate_only_bootvolume.go b/internal/controller/serverset/recreate_only_bootvolume.go index 08735e5f..2c9ae3db 100644 --- a/internal/controller/serverset/recreate_only_bootvolume.go +++ b/internal/controller/serverset/recreate_only_bootvolume.go @@ -19,6 +19,7 @@ func newCreateBeforeDestroyOnlyBootVolume(bootVolumeController kubeBootVolumeCon } func (c *createBeforeDestroyOnlyBootVolume) update(ctx context.Context, cr *v1alpha1.ServerSet, replicaIndex, volumeVersion, serverVersion int) error { + // todo we need the same IP in case of an update that deletes the bootvolume newVolumeVersion := volumeVersion + 1 if err := c.bootVolumeController.Ensure(ctx, cr, replicaIndex, newVolumeVersion); err != nil { return err diff --git a/internal/controller/serverset/serverset.go b/internal/controller/serverset/serverset.go index c81b9434..69302c5f 100644 --- a/internal/controller/serverset/serverset.go +++ b/internal/controller/serverset/serverset.go @@ -189,6 +189,10 @@ func (e *external) populateReplicasStatuses(ctx context.Context, cr *v1alpha1.Se } replicaIdx := ComputeReplicaIdx(e.log, fmt.Sprintf(indexLabel, cr.Name, ResourceServer), serverSetReplicas[i].Labels) + volumeVersion, err := getVolumeVersion(ctx, e.kube, cr.GetName(), replicaIdx) + if err != nil { + e.log.Info("error fetching volume version for", "name", cr.GetName(), "replicaIndex", replicaIdx, "error", err) + } nicStatues := computeNicStatuses(ctx, e, cr.Name, replicaIdx) cr.Status.AtProvider.ReplicaStatuses[i] = v1alpha1.ServerSetReplicaStatus{ Role: fetchRole(ctx, e, *cr, replicaIdx, serverSetReplicas[i].Name, replicaStatus), @@ -197,6 +201,7 @@ func (e *external) populateReplicasStatuses(ctx context.Context, cr *v1alpha1.Se NICStatuses: nicStatues, Status: replicaStatus, ErrorMessage: errMsg, + Hostname: getNameFrom(cr.Spec.ForProvider.Template.Metadata.Name, replicaIdx, volumeVersion), LastModified: metav1.Now(), } // for nfs we need to store substitutions in a configmap(created when the bootvolumes are created) and display them in the status @@ -647,41 +652,50 @@ var errNoVolumesFound = errors.New("no volumes found") // getVersionsFromVolumeAndServer checks that there is only one server and volume and returns their version func getVersionsFromVolumeAndServer(ctx context.Context, kube client.Client, serversetName string, replicaIndex int) (volumeVersion int, serverVersion int, err error) { - volumeResources := &v1alpha1.VolumeList{} - err = ListResFromSSetWithIndex(ctx, kube, serversetName, resourceBootVolume, replicaIndex, volumeResources) + volumeVersion, err = getVolumeVersion(ctx, kube, serversetName, replicaIndex) if err != nil { return volumeVersion, serverVersion, err } - if len(volumeResources.Items) > 1 { - return volumeVersion, serverVersion, fmt.Errorf("found too many volumes for index %d", replicaIndex) - } - if len(volumeResources.Items) == 0 { - return volumeVersion, serverVersion, fmt.Errorf("for index %d %w", replicaIndex, errNoVolumesFound) + + serverVersion, err = getServerVersion(ctx, kube, serversetName, replicaIndex) + if err != nil { + return volumeVersion, serverVersion, err } + return volumeVersion, serverVersion, nil +} + +func getServerVersion(ctx context.Context, kube client.Client, serversetName string, replicaIndex int) (int, error) { + serverVersion := 0 serverResources := &v1alpha1.ServerList{} - err = ListResFromSSetWithIndex(ctx, kube, serversetName, ResourceServer, replicaIndex, serverResources) + err := ListResFromSSetWithIndex(ctx, kube, serversetName, ResourceServer, replicaIndex, serverResources) if err != nil { - return volumeVersion, serverVersion, err + return serverVersion, err } if len(serverResources.Items) > 1 { - return volumeVersion, serverVersion, fmt.Errorf("found too many servers for index %d", replicaIndex) + return serverVersion, fmt.Errorf("found too many servers for index %d", replicaIndex) } if len(serverResources.Items) == 0 { - return volumeVersion, serverVersion, fmt.Errorf("found no servers for index %d", replicaIndex) + return serverVersion, fmt.Errorf("for index %d %w", replicaIndex, errNoVolumesFound) } + server := serverResources.Items[0] + return strconv.Atoi(server.Labels[fmt.Sprintf(versionLabel, serversetName, ResourceServer)]) +} - condemnedVolume := volumeResources.Items[0] - volumeVersion, err = strconv.Atoi(condemnedVolume.Labels[fmt.Sprintf(versionLabel, serversetName, resourceBootVolume)]) +func getVolumeVersion(ctx context.Context, kube client.Client, serversetName string, replicaIndex int) (int, error) { + volumeVersion := 0 + volumeResources := &v1alpha1.VolumeList{} + err := ListResFromSSetWithIndex(ctx, kube, serversetName, resourceBootVolume, replicaIndex, volumeResources) if err != nil { - return volumeVersion, serverVersion, err + return volumeVersion, err } - - servers := serverResources.Items - serverVersion, err = strconv.Atoi(servers[0].Labels[fmt.Sprintf(versionLabel, serversetName, ResourceServer)]) - if err != nil { - return volumeVersion, serverVersion, err + if len(volumeResources.Items) > 1 { + return volumeVersion, fmt.Errorf("found too many volumes for index %d", replicaIndex) } - return volumeVersion, serverVersion, nil + if len(volumeResources.Items) == 0 { + return volumeVersion, fmt.Errorf("for index %d %w", replicaIndex, errNoVolumesFound) + } + volume := volumeResources.Items[0] + return strconv.Atoi(volume.Labels[fmt.Sprintf(versionLabel, serversetName, resourceBootVolume)]) } func (e *external) ensureServerAndNicByIndex(ctx context.Context, cr *v1alpha1.ServerSet, replicaIndex, version int) error { diff --git a/internal/controller/serverset/serverset_test.go b/internal/controller/serverset/serverset_test.go index 0e2b1856..fef8ac4c 100644 --- a/internal/controller/serverset/serverset_test.go +++ b/internal/controller/serverset/serverset_test.go @@ -386,15 +386,15 @@ func Test_serverSetController_ServerSetObservation(t *testing.T) { cr *v1alpha1.ServerSet } - server1 := createServer("serverset-server-0-0") - server2 := createServer("serverset-server-1-0") + server1 := createServer("server-0-0") + server2 := createServer("server-1-0") label := fmt.Sprintf(indexLabel, serverSetName, ResourceServer) server2.Labels[label] = "1" serverWithErrorStatus := createServer("serverset-server-0-0") serverWithErrorStatus.Status.AtProvider.State = ionoscloud.Failed - serverWithUnknownStatus := createServer("serverset-server-0-0") + serverWithUnknownStatus := createServer("server-0-0") serverWithUnknownStatus.Status.AtProvider.State = "new-state" nic1 := createNic(v1alpha1.NicParameters{Name: server1.Name}) @@ -426,6 +426,7 @@ func Test_serverSetController_ServerSetObservation(t *testing.T) { ReplicaStatuses: []v1alpha1.ServerSetReplicaStatus{ { Name: server1.Name, + Hostname: getNameFrom(serverName, 0, 0), Status: statusReady, Role: "ACTIVE", ReplicaIndex: 0, @@ -441,6 +442,7 @@ func Test_serverSetController_ServerSetObservation(t *testing.T) { }, { Name: server2.Name, + Hostname: getNameFrom(serverName, 1, 0), Status: statusReady, Role: "PASSIVE", ReplicaIndex: 1, @@ -471,6 +473,7 @@ func Test_serverSetController_ServerSetObservation(t *testing.T) { ReplicaStatuses: []v1alpha1.ServerSetReplicaStatus{ { Name: server1.Name, + Hostname: getNameFrom(serverName, 0, 0), Status: statusReady, Role: "ACTIVE", ReplicaIndex: 0, @@ -486,6 +489,7 @@ func Test_serverSetController_ServerSetObservation(t *testing.T) { }, { Name: server2.Name, + Hostname: getNameFrom(serverName, 1, 0), Status: statusReady, Role: "PASSIVE", ReplicaIndex: 1, @@ -516,6 +520,7 @@ func Test_serverSetController_ServerSetObservation(t *testing.T) { ReplicaStatuses: []v1alpha1.ServerSetReplicaStatus{ { Name: server1.Name, + Hostname: getNameFrom(serverName, 0, 0), Status: statusReady, Role: "ACTIVE", ReplicaIndex: 0, @@ -531,6 +536,7 @@ func Test_serverSetController_ServerSetObservation(t *testing.T) { }, { Name: server2.Name, + Hostname: getNameFrom(serverName, 1, 0), Status: statusReady, Role: "PASSIVE", ReplicaIndex: 1, @@ -561,6 +567,7 @@ func Test_serverSetController_ServerSetObservation(t *testing.T) { ReplicaStatuses: []v1alpha1.ServerSetReplicaStatus{ { Name: server1.Name, + Hostname: getNameFrom(serverName, 0, 0), Status: statusReady, Role: "ACTIVE", ReplicaIndex: 0, @@ -578,6 +585,7 @@ func Test_serverSetController_ServerSetObservation(t *testing.T) { Name: server2.Name, Status: statusReady, Role: "PASSIVE", + Hostname: getNameFrom(serverName, 1, 0), ReplicaIndex: 1, NICStatuses: []v1alpha1.NicStatus{ { @@ -606,6 +614,7 @@ func Test_serverSetController_ServerSetObservation(t *testing.T) { ReplicaStatuses: []v1alpha1.ServerSetReplicaStatus{ { Name: server1.Name, + Hostname: getNameFrom(serverName, 0, 0), Status: statusReady, Role: "ACTIVE", ReplicaIndex: 0, @@ -638,6 +647,7 @@ func Test_serverSetController_ServerSetObservation(t *testing.T) { Name: serverWithErrorStatus.Name, Status: statusError, Role: "PASSIVE", + Hostname: getNameFrom(serverName, 0, 0), ReplicaIndex: 0, NICStatuses: []v1alpha1.NicStatus{}, ErrorMessage: "", @@ -659,6 +669,7 @@ func Test_serverSetController_ServerSetObservation(t *testing.T) { ReplicaStatuses: []v1alpha1.ServerSetReplicaStatus{ { Name: serverWithErrorStatus.Name, + Hostname: getNameFrom(serverName, 0, 0), Status: statusError, Role: "PASSIVE", ReplicaIndex: 0, @@ -682,6 +693,7 @@ func Test_serverSetController_ServerSetObservation(t *testing.T) { ReplicaStatuses: []v1alpha1.ServerSetReplicaStatus{ { Name: serverWithUnknownStatus.Name, + Hostname: getNameFrom(serverName, 0, 0), Status: statusUnknown, Role: "PASSIVE", ReplicaIndex: 0, @@ -711,9 +723,10 @@ func Test_serverSetController_ServerSetObservation(t *testing.T) { Replicas: 1, ReplicaStatuses: []v1alpha1.ServerSetReplicaStatus{ { - Name: serverNotReadyName, - Status: statusBusy, - Role: "PASSIVE", + Name: serverNotReadyName, + Hostname: getNameFrom(serverName, 0, 0), + Status: statusBusy, + Role: "PASSIVE", NICStatuses: []v1alpha1.NicStatus{ { AtProvider: v1alpha1.NicObservation{ diff --git a/package/crds/compute.ionoscloud.crossplane.io_serversets.yaml b/package/crds/compute.ionoscloud.crossplane.io_serversets.yaml index 8b858569..558958ea 100644 --- a/package/crds/compute.ionoscloud.crossplane.io_serversets.yaml +++ b/package/crds/compute.ionoscloud.crossplane.io_serversets.yaml @@ -715,6 +715,8 @@ spec: errorMessage: description: ErrorMessage relayed from the backend. type: string + hostname: + type: string lastModified: format: date-time type: string @@ -829,6 +831,7 @@ spec: type: string type: object required: + - hostname - name - replicaIndex - role diff --git a/package/crds/compute.ionoscloud.crossplane.io_statefulserversets.yaml b/package/crds/compute.ionoscloud.crossplane.io_statefulserversets.yaml index 1a9d88aa..1d363023 100644 --- a/package/crds/compute.ionoscloud.crossplane.io_statefulserversets.yaml +++ b/package/crds/compute.ionoscloud.crossplane.io_statefulserversets.yaml @@ -1042,6 +1042,8 @@ spec: errorMessage: description: ErrorMessage relayed from the backend. type: string + hostname: + type: string lastModified: format: date-time type: string @@ -1156,6 +1158,7 @@ spec: type: string type: object required: + - hostname - name - replicaIndex - role