Skip to content

Commit d9689da

Browse files
committed
fix e2e tests
Signed-off-by: Pankaj Walke <[email protected]>
1 parent ad7a804 commit d9689da

File tree

7 files changed

+99
-40
lines changed

7 files changed

+99
-40
lines changed

pkg/cloud/converters/eks.go

Lines changed: 4 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ package converters
2020
import (
2121
"errors"
2222
"fmt"
23-
"math"
2423

2524
"github.com/aws/aws-sdk-go-v2/aws"
2625
ekstypes "github.com/aws/aws-sdk-go-v2/service/eks/types"
@@ -188,21 +187,14 @@ func NodegroupUpdateconfigToSDK(updateConfig *expinfrav1.UpdateConfig) (*ekstype
188187
return nil, nil
189188
}
190189

191-
maxUnavailable, err := toSafeInt32(*updateConfig.MaxUnavailable)
192-
if err != nil {
193-
return nil, err
194-
}
195-
maxUnavailablePercant, err := toSafeInt32(*updateConfig.MaxUnavailablePercentage)
196-
if err != nil {
197-
return nil, err
198-
}
199-
200190
converted := &ekstypes.NodegroupUpdateConfig{}
201191
if updateConfig.MaxUnavailable != nil {
202-
converted.MaxUnavailable = aws.Int32(maxUnavailable)
192+
//nolint:gosec,G115 // Added golint exception as there is a kubebuilder validation configured
193+
converted.MaxUnavailable = aws.Int32(int32(*updateConfig.MaxUnavailable))
203194
}
204195
if updateConfig.MaxUnavailablePercentage != nil {
205-
converted.MaxUnavailablePercentage = aws.Int32(maxUnavailablePercant)
196+
//nolint:gosec,G115 // Added golint exception as there is a kubebuilder validation configured
197+
converted.MaxUnavailablePercentage = aws.Int32(int32(*updateConfig.MaxUnavailablePercentage))
206198
}
207199

208200
return converted, nil
@@ -256,10 +248,3 @@ func AddonConflictResolutionFromSDK(conflict ekstypes.ResolveConflicts) *string
256248
}
257249
return aws.String(string(ekscontrolplanev1.AddonResolutionOverwrite))
258250
}
259-
260-
func toSafeInt32(i int) (int32, error) {
261-
if i > math.MaxInt32 || i < math.MinInt32 {
262-
return 0, fmt.Errorf("value %d out of range for int32", i)
263-
}
264-
return int32(i), nil
265-
}

pkg/cloud/services/eks/cluster.go

Lines changed: 75 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,12 @@ import (
2424
"github.com/aws/aws-sdk-go-v2/aws"
2525
"github.com/aws/aws-sdk-go-v2/service/eks"
2626
ekstypes "github.com/aws/aws-sdk-go-v2/service/eks/types"
27+
"github.com/blang/semver"
2728
"github.com/pkg/errors"
2829
"k8s.io/apimachinery/pkg/util/sets"
2930
"k8s.io/apimachinery/pkg/util/version"
3031
"k8s.io/klog/v2"
32+
"k8s.io/utils/ptr"
3133

3234
infrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2"
3335
ekscontrolplanev1 "sigs.k8s.io/cluster-api-provider-aws/v2/controlplane/eks/api/v1beta2"
@@ -137,7 +139,54 @@ func (s *Service) reconcileCluster(ctx context.Context) error {
137139
return nil
138140
}
139141

142+
// computeCurrentStatusVersion returns the computed current EKS cluster kubernetes version.
143+
// The computation has awareness of the fact that EKS clusters only return a major.minor kubernetes version,
144+
// and returns a compatible version for te status according to the one the user specified in the spec.
145+
func computeCurrentStatusVersion(specV *string, clusterV *string) *string {
146+
specVersion := ""
147+
if specV != nil {
148+
specVersion = *specV
149+
}
150+
151+
clusterVersion := ""
152+
if clusterV != nil {
153+
clusterVersion = *clusterV
154+
}
155+
156+
// Ignore parsing errors as these are already validated by the kubebuilder validation and the AWS API.
157+
// Also specVersion might not be specified in the spec.Version for AWSManagedControlPlane, this results in a "0.0.0" version.
158+
// Also clusterVersion might not yet be returned by the AWS EKS API, as the cluster might still be initializing, this results in a "0.0.0" version.
159+
specSemverVersion, _ := semver.ParseTolerant(specVersion)
160+
currentSemverVersion, _ := semver.ParseTolerant(clusterVersion)
161+
162+
// If AWS EKS API is not returning a version, set the status.Version to empty string.
163+
if currentSemverVersion.String() == "0.0.0" {
164+
return ptr.To("")
165+
}
166+
167+
if currentSemverVersion.Major == specSemverVersion.Major &&
168+
currentSemverVersion.Minor == specSemverVersion.Minor &&
169+
specSemverVersion.Patch != 0 {
170+
// Treat this case differently as we want it to exactly match the spec.Version,
171+
// including its Patch, in the status.Version.
172+
currentSemverVersion.Patch = specSemverVersion.Patch
173+
174+
return ptr.To(currentSemverVersion.String())
175+
}
176+
177+
// For all the other cases it doesn't matter to have a patch version, as EKS ignores it internally.
178+
// So set the current cluster.Version (this always is a major.minor version format (e.g. "1.31")) in the status.Version.
179+
// Even in the event where in the spec.Version a zero patch version is specified (e.g. "1.31.0"),
180+
// the call to semver.ParseTolerant on the consumer side
181+
// will make sure the version with and without the trailing zero actually result in a match.
182+
return clusterV
183+
}
184+
140185
func (s *Service) setStatus(cluster *ekstypes.Cluster) error {
186+
// Set the current Kubernetes control plane version in the status.
187+
s.scope.ControlPlane.Status.Version = computeCurrentStatusVersion(s.scope.ControlPlane.Spec.Version, cluster.Version)
188+
189+
// Set the current cluster status in the control plane status.
141190
switch cluster.Status {
142191
case ekstypes.ClusterStatusDeleting:
143192
s.scope.ControlPlane.Status.Ready = false
@@ -521,16 +570,35 @@ func (s *Service) reconcileLogging(ctx context.Context, logging *ekstypes.Loggin
521570
return nil
522571
}
523572

524-
func publicAccessCIDRsEqual(as []*string, bs []*string) bool {
525-
all := "0.0.0.0/0"
573+
func publicAccessCIDRsEqual(as []string, bs []string) bool {
574+
allV4 := "0.0.0.0/0"
575+
allV6 := "::/0"
576+
asDefault := false
577+
bsDefault := false
578+
579+
// For IPv6 only clusters
526580
if len(as) == 0 {
527-
as = []*string{&all}
581+
as = []string{allV4, allV6}
582+
asDefault = true
528583
}
529584
if len(bs) == 0 {
530-
bs = []*string{&all}
585+
bs = []string{allV4, allV6}
586+
bsDefault = true
587+
}
588+
if sets.NewString(as...).Equal(sets.NewString(bs...)) {
589+
fmt.Println("Found IPV6 true")
590+
return true
591+
}
592+
593+
// For IPv4 only clusters
594+
if asDefault {
595+
as = []string{allV4}
596+
}
597+
if bsDefault {
598+
bs = []string{allV4}
531599
}
532-
return sets.NewString(aws.ToStringSlice(as)...).Equal(
533-
sets.NewString(aws.ToStringSlice(bs)...),
600+
return sets.NewString(as...).Equal(
601+
sets.NewString(bs...),
534602
)
535603
}
536604

@@ -550,7 +618,7 @@ func (s *Service) reconcileVpcConfig(vpcConfig *ekstypes.VpcConfigResponse) (*ek
550618
}
551619
needsUpdate := !tristate.EqualWithDefault(false, &vpcConfig.EndpointPrivateAccess, updatedVpcConfig.EndpointPrivateAccess) ||
552620
!tristate.EqualWithDefault(true, &vpcConfig.EndpointPublicAccess, updatedVpcConfig.EndpointPublicAccess) ||
553-
!publicAccessCIDRsEqual(aws.StringSlice(vpcConfig.PublicAccessCidrs), aws.StringSlice(updatedVpcConfig.PublicAccessCidrs))
621+
!publicAccessCIDRsEqual(vpcConfig.PublicAccessCidrs, updatedVpcConfig.PublicAccessCidrs)
554622
if needsUpdate {
555623
return &ekstypes.VpcConfigRequest{
556624
EndpointPublicAccess: updatedVpcConfig.EndpointPublicAccess,

pkg/cloud/services/eks/cluster_test.go

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -283,8 +283,8 @@ func TestMakeVPCConfig(t *testing.T) {
283283
func TestPublicAccessCIDRsEqual(t *testing.T) {
284284
testCases := []struct {
285285
name string
286-
a []*string
287-
b []*string
286+
a []string
287+
b []string
288288
expect bool
289289
}{
290290
{
@@ -294,15 +294,21 @@ func TestPublicAccessCIDRsEqual(t *testing.T) {
294294
expect: true,
295295
},
296296
{
297-
name: "every address",
298-
a: []*string{aws.String("0.0.0.0/0")},
297+
name: "every ipv4 address",
298+
a: []string{"0.0.0.0/0"},
299+
b: nil,
300+
expect: true,
301+
},
302+
{
303+
name: "every ipv4 and ipv6 address",
304+
a: []string{"0.0.0.0/0", "::/0"},
299305
b: nil,
300306
expect: true,
301307
},
302308
{
303309
name: "every address",
304-
a: []*string{aws.String("1.1.1.0/24")},
305-
b: []*string{aws.String("1.1.1.0/24")},
310+
a: []string{"1.1.1.0/24"},
311+
b: []string{"1.1.1.0/24"},
306312
expect: true,
307313
},
308314
}

test/e2e/shared/aws.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -442,7 +442,7 @@ func NewAWSSessionV2() *awsv2.Config {
442442
Expect(err).NotTo(HaveOccurred())
443443
optFns := []func(*config.LoadOptions) error{
444444
config.WithRegion(region),
445-
config.WithClientLogMode(awsv2.LogSigning),
445+
config.WithClientLogMode(awsv2.LogRequest),
446446
}
447447
cfg, err := config.LoadDefaultConfig(context.Background(), optFns...)
448448
Expect(err).NotTo(HaveOccurred())
@@ -458,7 +458,7 @@ func NewAWSSessionRepoWithKeyV2(accessKey *iamtypes.AccessKey) *awsv2.Config {
458458
staticCredProvider := awscredsv2.NewStaticCredentialsProvider(awsv2.ToString(accessKey.AccessKeyId), awsv2.ToString(accessKey.SecretAccessKey), "")
459459
optFns := []func(*config.LoadOptions) error{
460460
config.WithRegion(region),
461-
config.WithClientLogMode(awsv2.LogSigning),
461+
config.WithClientLogMode(awsv2.LogRequest),
462462
config.WithCredentialsProvider(staticCredProvider),
463463
}
464464
cfg, err := config.LoadDefaultConfig(context.Background(), optFns...)
@@ -475,7 +475,7 @@ func NewAWSSessionWithKeyV2(accessKey *iamtypes.AccessKey) *awsv2.Config {
475475
staticCredProvider := awscredsv2.NewStaticCredentialsProvider(awsv2.ToString(accessKey.AccessKeyId), awsv2.ToString(accessKey.SecretAccessKey), "")
476476
optFns := []func(*config.LoadOptions) error{
477477
config.WithRegion(region),
478-
config.WithClientLogMode(awsv2.LogSigning),
478+
config.WithClientLogMode(awsv2.LogRequest),
479479
config.WithCredentialsProvider(staticCredProvider),
480480
}
481481
cfg, err := config.LoadDefaultConfig(context.Background(), optFns...)

test/e2e/suites/managed/eks_ipv6_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ var _ = ginkgo.Describe("[managed] [general] [ipv6] EKS cluster tests", func() {
8383
ConfigClusterFn: defaultConfigCluster,
8484
BootstrapClusterProxy: e2eCtx.Environment.BootstrapClusterProxy,
8585
AWSSession: e2eCtx.BootstrapUserAWSSession,
86-
AWSConfig: e2eCtx.AWSConfig,
86+
AWSSessionV2: e2eCtx.BootstrapUserAWSSessionV2,
8787
Namespace: namespace,
8888
ClusterName: clusterName,
8989
IncludeScaling: false,

test/e2e/suites/managed/eks_legacy_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ var _ = ginkgo.Describe("[managed] [legacy] EKS cluster tests - single kind", fu
8383
ConfigClusterFn: defaultConfigCluster,
8484
BootstrapClusterProxy: e2eCtx.Environment.BootstrapClusterProxy,
8585
AWSSession: e2eCtx.BootstrapUserAWSSession,
86-
AWSConfig: e2eCtx.AWSConfig,
86+
AWSSessionV2: e2eCtx.BootstrapUserAWSSessionV2,
8787
Namespace: namespace,
8888
ClusterName: clusterName,
8989
IncludeScaling: false,

test/e2e/suites/managed/eks_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ var _ = ginkgo.Describe("[managed] [general] EKS cluster tests", func() {
119119
ConfigClusterFn: defaultConfigCluster,
120120
BootstrapClusterProxy: e2eCtx.Environment.BootstrapClusterProxy,
121121
AWSSession: e2eCtx.BootstrapUserAWSSession,
122-
AWSConfig: e2eCtx.AWSConfig,
122+
AWSSessionV2: e2eCtx.BootstrapUserAWSSessionV2,
123123
Namespace: namespace,
124124
ClusterName: clusterName,
125125
IncludeScaling: true,
@@ -136,7 +136,7 @@ var _ = ginkgo.Describe("[managed] [general] EKS cluster tests", func() {
136136
ConfigClusterFn: defaultConfigCluster,
137137
BootstrapClusterProxy: e2eCtx.Environment.BootstrapClusterProxy,
138138
AWSSession: e2eCtx.BootstrapUserAWSSession,
139-
AWSConfig: e2eCtx.AWSConfig,
139+
AWSSessionV2: e2eCtx.BootstrapUserAWSSessionV2,
140140
Namespace: namespace,
141141
ClusterName: clusterName,
142142
IncludeScaling: true,
@@ -154,7 +154,7 @@ var _ = ginkgo.Describe("[managed] [general] EKS cluster tests", func() {
154154
ConfigClusterFn: defaultConfigCluster,
155155
BootstrapClusterProxy: e2eCtx.Environment.BootstrapClusterProxy,
156156
AWSSession: e2eCtx.BootstrapUserAWSSession,
157-
AWSConfig: e2eCtx.AWSConfig,
157+
AWSSessionV2: e2eCtx.BootstrapUserAWSSessionV2,
158158
Namespace: namespace,
159159
ClusterName: clusterName,
160160
IncludeScaling: true,

0 commit comments

Comments
 (0)