Skip to content

Commit

Permalink
Fix inconsistent result after creating SKE cluster with empty node po… (
Browse files Browse the repository at this point in the history 
#461)

* Fix inconsistent result after creating SKE cluster with empty node pool taints

* Improve code readability

* Suggestions from review
  • Loading branch information
@joaopalet
joaopalet authored Jul 11, 2024
1 parent a15bd14 commit a9c50fc
Show file tree
Hide file tree
Showing 2 changed files with 217 additions and 2 deletions.
16 changes: 14 additions & 2 deletions stackit/internal/services/ske/cluster/resource.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1323,7 +1323,11 @@ func mapNodePools(ctx context.Context, cl *ske.Cluster, m *Model) error {
nodePool["cri"] = types.StringPointerValue(nodePoolResp.Cri.Name)
}

err := mapTaints(nodePoolResp.Taints, nodePool)
taintsInModel := false
if i < len(modelNodePools) && !modelNodePools[i].Taints.IsNull() && !modelNodePools[i].Taints.IsUnknown() {
taintsInModel = true
}
err := mapTaints(nodePoolResp.Taints, nodePool, taintsInModel)
if err != nil {
return fmt.Errorf("mapping index %d, field taints: %w", i, err)
}
Expand Down Expand Up @@ -1362,8 +1366,16 @@ func mapNodePools(ctx context.Context, cl *ske.Cluster, m *Model) error {
return nil
}

func mapTaints(t *[]ske.Taint, nodePool map[string]attr.Value) error {
func mapTaints(t *[]ske.Taint, nodePool map[string]attr.Value, existInModel bool) error {
if t == nil || len(*t) == 0 {
if existInModel {
taintsTF, diags := types.ListValue(types.ObjectType{AttrTypes: taintTypes}, []attr.Value{})
if diags.HasError() {
return fmt.Errorf("create empty taints list: %w", core.DiagsToError(diags))
}
nodePool["taints"] = taintsTF
return nil
}
nodePool["taints"] = types.ListNull(types.ObjectType{AttrTypes: taintTypes})
return nil
}
Expand Down
203 changes: 203 additions & 0 deletions stackit/internal/services/ske/cluster/resource_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,15 @@ func TestMapFields(t *testing.T) {
tests := []struct {
description string
stateExtensions types.Object
stateNodePools types.List
input *ske.Cluster
expected Model
isValid bool
}{
{
"default_values",
types.ObjectNull(extensionsTypes),
types.ListNull(types.ObjectType{AttrTypes: nodePoolTypes}),
&ske.Cluster{
Name: utils.Ptr("name"),
},
Expand All @@ -59,6 +61,7 @@ func TestMapFields(t *testing.T) {
{
"simple_values",
types.ObjectNull(extensionsTypes),
types.ListNull(types.ObjectType{AttrTypes: nodePoolTypes}),
&ske.Cluster{
Extensions: &ske.Extension{
Acl: &ske.ACL{
Expand Down Expand Up @@ -233,6 +236,7 @@ func TestMapFields(t *testing.T) {
{
"empty_network",
types.ObjectNull(extensionsTypes),
types.ListNull(types.ObjectType{AttrTypes: nodePoolTypes}),
&ske.Cluster{
Name: utils.Ptr("name"),
Network: &ske.Network{},
Expand All @@ -255,6 +259,7 @@ func TestMapFields(t *testing.T) {
{
"extensions_mixed_values",
types.ObjectNull(extensionsTypes),
types.ListNull(types.ObjectType{AttrTypes: nodePoolTypes}),
&ske.Cluster{
Extensions: &ske.Extension{
Acl: &ske.ACL{
Expand Down Expand Up @@ -303,6 +308,7 @@ func TestMapFields(t *testing.T) {
"argus_instance_id": types.StringNull(),
}),
}),
types.ListNull(types.ObjectType{AttrTypes: nodePoolTypes}),
&ske.Cluster{
Extensions: &ske.Extension{},
Name: utils.Ptr("name"),
Expand Down Expand Up @@ -344,6 +350,7 @@ func TestMapFields(t *testing.T) {
"argus_instance_id": types.StringValue("id"),
}),
}),
types.ListNull(types.ObjectType{AttrTypes: nodePoolTypes}),
&ske.Cluster{
Extensions: &ske.Extension{
Acl: &ske.ACL{
Expand Down Expand Up @@ -381,6 +388,7 @@ func TestMapFields(t *testing.T) {
{
"extensions_not_set",
types.ObjectNull(extensionsTypes),
types.ListNull(types.ObjectType{AttrTypes: nodePoolTypes}),
&ske.Cluster{
Extensions: &ske.Extension{},
Name: utils.Ptr("name"),
Expand All @@ -399,16 +407,210 @@ func TestMapFields(t *testing.T) {
},
true,
},
{
"nil_taints_when_empty_list_on_state",
types.ObjectNull(extensionsTypes),
types.ListValueMust(
types.ObjectType{AttrTypes: nodePoolTypes},
[]attr.Value{
types.ObjectValueMust(
nodePoolTypes,
map[string]attr.Value{
"name": types.StringValue("node"),
"machine_type": types.StringValue("B"),
"os_name": types.StringValue("os"),
"os_version": types.StringNull(),
"os_version_min": types.StringNull(),
"os_version_used": types.StringValue("os-ver"),
"minimum": types.Int64Value(1),
"maximum": types.Int64Value(5),
"max_surge": types.Int64Value(3),
"max_unavailable": types.Int64Null(),
"volume_type": types.StringValue("type"),
"volume_size": types.Int64Value(3),
"labels": types.MapValueMust(
types.StringType,
map[string]attr.Value{
"k": types.StringValue("v"),
},
),
"taints": types.ListValueMust(types.ObjectType{AttrTypes: taintTypes}, []attr.Value{}),
"cri": types.StringValue("cri"),
"availability_zones": types.ListValueMust(
types.StringType,
[]attr.Value{
types.StringValue("z1"),
types.StringValue("z2"),
},
),
},
),
},
),
&ske.Cluster{
Extensions: &ske.Extension{
Acl: &ske.ACL{
AllowedCidrs: &[]string{"cidr1"},
Enabled: utils.Ptr(true),
},
Argus: &ske.Argus{
ArgusInstanceId: utils.Ptr("aid"),
Enabled: utils.Ptr(true),
},
},
Hibernation: &ske.Hibernation{
Schedules: &[]ske.HibernationSchedule{
{
End: utils.Ptr("2"),
Start: utils.Ptr("1"),
Timezone: utils.Ptr("CET"),
},
},
},
Kubernetes: &ske.Kubernetes{
AllowPrivilegedContainers: utils.Ptr(true),
Version: utils.Ptr("1.2.3"),
},
Maintenance: &ske.Maintenance{
AutoUpdate: &ske.MaintenanceAutoUpdate{
KubernetesVersion: utils.Ptr(true),
MachineImageVersion: utils.Ptr(true),
},
TimeWindow: &ske.TimeWindow{
Start: utils.Ptr("0000-01-02T03:04:05+06:00"),
End: utils.Ptr("0010-11-12T13:14:15Z"),
},
},
Network: &ske.Network{
Id: utils.Ptr("nid"),
},
Name: utils.Ptr("name"),
Nodepools: &[]ske.Nodepool{
{
AvailabilityZones: &[]string{"z1", "z2"},
Cri: &ske.CRI{
Name: utils.Ptr("cri"),
},
Labels: &map[string]string{"k": "v"},
Machine: &ske.Machine{
Image: &ske.Image{
Name: utils.Ptr("os"),
Version: utils.Ptr("os-ver"),
},
Type: utils.Ptr("B"),
},
MaxSurge: utils.Ptr(int64(3)),
MaxUnavailable: nil,
Maximum: utils.Ptr(int64(5)),
Minimum: utils.Ptr(int64(1)),
Name: utils.Ptr("node"),
Taints: nil,
Volume: &ske.Volume{
Size: utils.Ptr(int64(3)),
Type: utils.Ptr("type"),
},
},
},
Status: &ske.ClusterStatus{
Aggregated: &cs,
Error: nil,
Hibernated: nil,
},
},
Model{
Id: types.StringValue("pid,name"),
ProjectId: types.StringValue("pid"),
Name: types.StringValue("name"),
KubernetesVersion: types.StringNull(),
KubernetesVersionUsed: types.StringValue("1.2.3"),
AllowPrivilegedContainers: types.BoolValue(true),
NodePools: types.ListValueMust(
types.ObjectType{AttrTypes: nodePoolTypes},
[]attr.Value{
types.ObjectValueMust(
nodePoolTypes,
map[string]attr.Value{
"name": types.StringValue("node"),
"machine_type": types.StringValue("B"),
"os_name": types.StringValue("os"),
"os_version": types.StringNull(),
"os_version_min": types.StringNull(),
"os_version_used": types.StringValue("os-ver"),
"minimum": types.Int64Value(1),
"maximum": types.Int64Value(5),
"max_surge": types.Int64Value(3),
"max_unavailable": types.Int64Null(),
"volume_type": types.StringValue("type"),
"volume_size": types.Int64Value(3),
"labels": types.MapValueMust(
types.StringType,
map[string]attr.Value{
"k": types.StringValue("v"),
},
),
"taints": types.ListValueMust(types.ObjectType{AttrTypes: taintTypes}, []attr.Value{}),
"cri": types.StringValue("cri"),
"availability_zones": types.ListValueMust(
types.StringType,
[]attr.Value{
types.StringValue("z1"),
types.StringValue("z2"),
},
),
},
),
},
),
Maintenance: types.ObjectValueMust(maintenanceTypes, map[string]attr.Value{
"enable_kubernetes_version_updates": types.BoolValue(true),
"enable_machine_image_version_updates": types.BoolValue(true),
"start": types.StringValue("03:04:05+06:00"),
"end": types.StringValue("13:14:15Z"),
}),
Network: types.ObjectValueMust(networkTypes, map[string]attr.Value{
"id": types.StringValue("nid"),
}),
Hibernations: types.ListValueMust(
types.ObjectType{AttrTypes: hibernationTypes},
[]attr.Value{
types.ObjectValueMust(
hibernationTypes,
map[string]attr.Value{
"start": types.StringValue("1"),
"end": types.StringValue("2"),
"timezone": types.StringValue("CET"),
},
),
},
),
Extensions: types.ObjectValueMust(extensionsTypes, map[string]attr.Value{
"acl": types.ObjectValueMust(aclTypes, map[string]attr.Value{
"enabled": types.BoolValue(true),
"allowed_cidrs": types.ListValueMust(types.StringType, []attr.Value{
types.StringValue("cidr1"),
}),
}),
"argus": types.ObjectValueMust(argusTypes, map[string]attr.Value{
"enabled": types.BoolValue(true),
"argus_instance_id": types.StringValue("aid"),
}),
}),
KubeConfig: types.StringNull(),
},
true,
},
{
"nil_response",
types.ObjectNull(extensionsTypes),
types.ListNull(types.ObjectType{AttrTypes: nodePoolTypes}),
nil,
Model{},
false,
},
{
"no_resource_id",
types.ObjectNull(extensionsTypes),
types.ListNull(types.ObjectType{AttrTypes: nodePoolTypes}),
&ske.Cluster{},
Model{},
false,
Expand All @@ -419,6 +621,7 @@ func TestMapFields(t *testing.T) {
state := &Model{
ProjectId: tt.expected.ProjectId,
Extensions: tt.stateExtensions,
NodePools: tt.stateNodePools,
}
err := mapFields(context.Background(), tt.input, state)
if !tt.isValid && err == nil {
Expand Down

0 comments on commit a9c50fc

Please sign in to comment.