From 062a0bc940326881b45ff8f51d868d2b6a9007e2 Mon Sep 17 00:00:00 2001 From: Sebastian Cole Date: Thu, 21 Sep 2023 11:13:30 +1000 Subject: [PATCH] Address linting errors --- pkg/apis/settings/zz_generated.deepcopy.go | 1 - pkg/apis/v1alpha1/zz_generated.deepcopy.go | 1 - pkg/apis/v1alpha5/zz_generated.deepcopy.go | 1 - pkg/apis/v1beta1/zz_generated.deepcopy.go | 1 - pkg/controllers/nodeclass/controller.go | 9 +++------ pkg/fake/lmapi.go | 3 +-- pkg/fake/rgapi.go | 8 ++++---- pkg/providers/amifamily/resolver.go | 20 +++++++++----------- pkg/test/environment.go | 6 +++--- pkg/utils/nodeclass/suite_test.go | 12 ++++++------ 10 files changed, 26 insertions(+), 36 deletions(-) diff --git a/pkg/apis/settings/zz_generated.deepcopy.go b/pkg/apis/settings/zz_generated.deepcopy.go index 43eaa173bfe6..51a3bd930dd9 100644 --- a/pkg/apis/settings/zz_generated.deepcopy.go +++ b/pkg/apis/settings/zz_generated.deepcopy.go @@ -1,5 +1,4 @@ //go:build !ignore_autogenerated -// +build !ignore_autogenerated /* Licensed under the Apache License, Version 2.0 (the "License"); diff --git a/pkg/apis/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/v1alpha1/zz_generated.deepcopy.go index 15b0b9d21ac8..55f62e53a512 100644 --- a/pkg/apis/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/v1alpha1/zz_generated.deepcopy.go @@ -1,5 +1,4 @@ //go:build !ignore_autogenerated -// +build !ignore_autogenerated /* Licensed under the Apache License, Version 2.0 (the "License"); diff --git a/pkg/apis/v1alpha5/zz_generated.deepcopy.go b/pkg/apis/v1alpha5/zz_generated.deepcopy.go index e55c4cb4b353..722b9872175f 100644 --- a/pkg/apis/v1alpha5/zz_generated.deepcopy.go +++ b/pkg/apis/v1alpha5/zz_generated.deepcopy.go @@ -1,5 +1,4 @@ //go:build !ignore_autogenerated -// +build !ignore_autogenerated /* Licensed under the Apache License, Version 2.0 (the "License"); diff --git a/pkg/apis/v1beta1/zz_generated.deepcopy.go b/pkg/apis/v1beta1/zz_generated.deepcopy.go index 34ebcc10daf0..66b99865f9a0 100644 --- a/pkg/apis/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/v1beta1/zz_generated.deepcopy.go @@ -1,5 +1,4 @@ //go:build !ignore_autogenerated -// +build !ignore_autogenerated /* Licensed under the Apache License, Version 2.0 (the "License"); diff --git a/pkg/controllers/nodeclass/controller.go b/pkg/controllers/nodeclass/controller.go index c5be9a030912..61dcb5909378 100644 --- a/pkg/controllers/nodeclass/controller.go +++ b/pkg/controllers/nodeclass/controller.go @@ -169,8 +169,7 @@ func (c *Controller) resolveAMIs(ctx context.Context, nodeClass *v1beta1.EC2Node func (c *Controller) resolveLicenses(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) error { licenses, err := c.licenseProvider.Get(ctx, nodeClass) if err != nil { - // Errors from license provider should not interrupt the process - return nil + return err } nodeClass.Status.Licenses = licenses @@ -181,8 +180,7 @@ func (c *Controller) resolveLicenses(ctx context.Context, nodeClass *v1beta1.EC2 func (c *Controller) resolveHostResourceGroups(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) error { result, err := c.hostResourceGroupProvider.Get(ctx, nodeClass) if err != nil { - // Errors from host resource group provider should not interrupt the process - return nil + return err } nodeClass.Status.HostResourceGroup = result @@ -193,8 +191,7 @@ func (c *Controller) resolveHostResourceGroups(ctx context.Context, nodeClass *v func (c *Controller) resolvePlacementGroups(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) error { result, err := c.placementGroupProvider.Get(ctx, nodeClass) if err != nil { - // Errors from placement group provider should not interrupt the process - return nil + return err } if result != nil { nodeClass.Status.PlacementGroups = append(nodeClass.Status.PlacementGroups, *result.GroupArn) diff --git a/pkg/fake/lmapi.go b/pkg/fake/lmapi.go index 96277bc1662b..88f0695859fd 100644 --- a/pkg/fake/lmapi.go +++ b/pkg/fake/lmapi.go @@ -28,7 +28,7 @@ type LicenseManagerAPI struct { LicenseManagerBehaviour } type LicenseManagerBehaviour struct { - NextError AtomicError + NextError AtomicError ListLicenseConfigurationsOutput AtomicPtr[licensemanager.ListLicenseConfigurationsOutput] } @@ -48,4 +48,3 @@ func (l *LicenseManagerAPI) ListLicenseConfigurations(_ aws.Context, _ *licensem // fail if the test doesn't provide specific data which causes our pricing provider to use its static price list return errors.New("no license data provided") } - diff --git a/pkg/fake/rgapi.go b/pkg/fake/rgapi.go index f0b4141352f9..082d2df8f06e 100644 --- a/pkg/fake/rgapi.go +++ b/pkg/fake/rgapi.go @@ -24,8 +24,8 @@ import ( ) var ( - TEST_LICENSE_ARN = "arn:aws:license-manager:us-west-2:11111111111:license-configuration:lic-94ba36399bd98eaad808b0ffb1d1604b" - TEST_LICENSE_NAME = "test-license" + TestLicenseArn = "arn:aws:license-manager:us-west-2:11111111111:license-configuration:lic-94ba36399bd98eaad808b0ffb1d1604b" + TestLicenseName = "test-license" ) type ResourceGroupsAPI struct { @@ -43,11 +43,11 @@ func (r *ResourceGroupsAPI) Reset() { r.ListGroupsOutput.Reset() } -func (r *ResourceGroupsAPI) ListGroupsWithContext(_ aws.Context, input *resourcegroups.ListGroupsInput, opts ...request.Option) (*resourcegroups.ListGroupsOutput, error) { +func (r *ResourceGroupsAPI) ListGroupsWithContext(_ aws.Context, input *resourcegroups.ListGroupsInput, _ ...request.Option) (*resourcegroups.ListGroupsOutput, error) { return r.ListGroupsBehavior.Invoke(input, func(input *resourcegroups.ListGroupsInput) (*resourcegroups.ListGroupsOutput, error) { return &resourcegroups.ListGroupsOutput{ GroupIdentifiers: []*resourcegroups.GroupIdentifier{ - {GroupArn: aws.String(TEST_LICENSE_ARN), GroupName: aws.String(TEST_LICENSE_NAME)}, + {GroupArn: aws.String(TestLicenseArn), GroupName: aws.String(TestLicenseName)}, }, }, nil }) diff --git a/pkg/providers/amifamily/resolver.go b/pkg/providers/amifamily/resolver.go index e120e8bfad88..8d004efa78bd 100644 --- a/pkg/providers/amifamily/resolver.go +++ b/pkg/providers/amifamily/resolver.go @@ -145,10 +145,6 @@ func (r Resolver) Resolve(ctx context.Context, nodeClass *v1beta1.EC2NodeClass, if len(mappedAMIs) == 0 { return nil, fmt.Errorf("no instance types satisfy requirements of amis %v", amis) } - placement, err := r.resolvePlacement(ctx, nodeClass) - if err != nil { - return nil, err - } var resolvedTemplates []*LaunchTemplate for amiID, instanceTypes := range mappedAMIs { maxPodsToInstanceTypes := lo.GroupBy(instanceTypes, func(instanceType *cloudprovider.InstanceType) int { @@ -183,7 +179,7 @@ func (r Resolver) Resolve(ctx context.Context, nodeClass *v1beta1.EC2NodeClass, AMIID: amiID, InstanceTypes: instanceTypes, Licenses: nodeClass.Status.Licenses, - Placement: placement, + Placement: r.resolvePlacement(nodeClass), } if len(resolved.BlockDeviceMappings) == 0 { resolved.BlockDeviceMappings = amiFamily.DefaultBlockDeviceMappings() @@ -197,21 +193,23 @@ func (r Resolver) Resolve(ctx context.Context, nodeClass *v1beta1.EC2NodeClass, return resolvedTemplates, nil } -func (r Resolver) resolvePlacement(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) (*Placement, error) { +func (r Resolver) resolvePlacement(nodeClass *v1beta1.EC2NodeClass) *Placement { var placement *Placement hrg := nodeClass.Status.HostResourceGroup pg := nodeClass.Status.PlacementGroups - if pg != nil { + if pg != nil || hrg != nil { + placement = &Placement{} if len(pg) > 0 { placement.PlacementGroup = pg[0] } - } - if hrg != nil { - placement.HostResourceGroup = hrg.Name + + if hrg != nil { + placement.HostResourceGroup = hrg.Name + } } - return placement, nil + return placement } func GetAMIFamily(amiFamily *string, options *Options) AMIFamily { diff --git a/pkg/test/environment.go b/pkg/test/environment.go index 64d43d0ed32a..d14bacde7467 100644 --- a/pkg/test/environment.go +++ b/pkg/test/environment.go @@ -81,7 +81,7 @@ func NewEnvironment(ctx context.Context, env *coretest.Environment) *Environment ec2api := &fake.EC2API{} ssmapi := &fake.SSMAPI{} lmapi := &fake.LicenseManagerAPI{} - rgapi := &fake.ResourceGroupsAPI{} + rgapi := &fake.ResourceGroupsAPI{} // cache ec2Cache := cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval) @@ -103,8 +103,8 @@ func NewEnvironment(ctx context.Context, env *coretest.Environment) *Environment versionProvider := version.NewProvider(env.KubernetesInterface, kubernetesVersionCache) amiProvider := amifamily.NewProvider(versionProvider, ssmapi, ec2api, ec2Cache) licenseProvider := license.NewProvider(lmapi.LicenseManagerAPI, licenseCache) - hostResourceGroupProvider := hostresourcegroup.NewProvider(rgapi, hostResourceGroupsCache) - placementGroupProvider := placementgroup.NewProvider(ec2api, placementGroupCache) + hostResourceGroupProvider := hostresourcegroup.NewProvider(rgapi, hostResourceGroupsCache) + placementGroupProvider := placementgroup.NewProvider(ec2api, placementGroupCache) amiResolver := amifamily.New(amiProvider, licenseProvider, hostResourceGroupProvider, placementGroupProvider) instanceTypesProvider := instancetype.NewProvider("", instanceTypeCache, ec2api, subnetProvider, unavailableOfferingsCache, pricingProvider) diff --git a/pkg/utils/nodeclass/suite_test.go b/pkg/utils/nodeclass/suite_test.go index 77be7a488cf6..8277c31769d7 100644 --- a/pkg/utils/nodeclass/suite_test.go +++ b/pkg/utils/nodeclass/suite_test.go @@ -434,13 +434,13 @@ var _ = Describe("NodeClassUtils", func() { Expect(convertedNodeTemplate.Status.Subnets).To(Equal(nodeTemplate.Status.Subnets)) Expect(convertedNodeTemplate.Status.AMIs).To(Equal(nodeTemplate.Status.AMIs)) }) - It("should convert a AWSNodeTemplate with LicenseSelectors to a EC2NodeClass and back", func() { - nodeTemplate.Status.Licenses = []string{"test-license"} - nodeTemplate.Spec.LicenseSelector = map[string]string{"name": "test-license"} + It("should convert a AWSNodeTemplate with LicenseSelectors to a EC2NodeClass and back", func() { + nodeTemplate.Status.Licenses = []string{"test-license"} + nodeTemplate.Spec.LicenseSelector = map[string]string{"name": "test-license"} convertedNodeTemplate := nodetemplateutil.New(nodeclassutil.New(nodeTemplate)) - Expect(convertedNodeTemplate.Status.Licenses).To(Equal(nodeTemplate.Status.Licenses)) - Expect(convertedNodeTemplate.Spec.LicenseSelector).To(Equal(nodeTemplate.Spec.LicenseSelector)) - }) + Expect(convertedNodeTemplate.Status.Licenses).To(Equal(nodeTemplate.Status.Licenses)) + Expect(convertedNodeTemplate.Spec.LicenseSelector).To(Equal(nodeTemplate.Spec.LicenseSelector)) + }) It("should retrieve a EC2NodeClass with a get call", func() { nodeClass := test.EC2NodeClass() ExpectApplied(ctx, env.Client, nodeClass)