From 16bbca7ce94182f11d5582f600f2d542b686c719 Mon Sep 17 00:00:00 2001 From: Jonathan Innis Date: Fri, 10 May 2024 14:51:18 -0500 Subject: [PATCH] fix: Ensure shallow copy of data when returning back cached data (#6167) --- .../karpenter.k8s.aws_awsnodetemplates.yaml | 268 +++++++++------ .../karpenter.k8s.aws_ec2nodeclasses.yaml | 322 ++++++++++-------- pkg/providers/amifamily/ami.go | 10 +- pkg/providers/amifamily/ami_test.go | 53 ++- pkg/providers/instancetype/instancetype.go | 4 +- pkg/providers/instancetype/nodeclass_test.go | 36 ++ .../instancetype/nodetemplate_test.go | 36 ++ pkg/providers/securitygroup/securitygroup.go | 4 +- pkg/providers/securitygroup/suite_test.go | 68 ++++ pkg/providers/subnet/subnet.go | 4 +- pkg/providers/subnet/suite_test.go | 77 +++++ 11 files changed, 628 insertions(+), 254 deletions(-) diff --git a/pkg/apis/crds/karpenter.k8s.aws_awsnodetemplates.yaml b/pkg/apis/crds/karpenter.k8s.aws_awsnodetemplates.yaml index 20cf10f8b3d2..f68a599b7649 100644 --- a/pkg/apis/crds/karpenter.k8s.aws_awsnodetemplates.yaml +++ b/pkg/apis/crds/karpenter.k8s.aws_awsnodetemplates.yaml @@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.13.0 + controller-gen.kubebuilder.io/version: v0.15.0 name: awsnodetemplates.karpenter.k8s.aws spec: group: karpenter.k8s.aws @@ -22,21 +22,26 @@ spec: description: AWSNodeTemplate is the Schema for the AWSNodeTemplate API properties: apiVersion: - description: 'APIVersion defines the versioned schema of this representation - of an object. Servers should convert recognized schemas to the latest - internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources type: string kind: - description: 'Kind is a string value representing the REST resource this - object represents. Servers may infer this from the endpoint the client - submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds type: string metadata: type: object spec: - description: AWSNodeTemplateSpec is the top level specification for the - AWS Karpenter Provider. This will contain configuration necessary to - launch instances in AWS. + description: |- + AWSNodeTemplateSpec is the top level specification for the AWS Karpenter Provider. + This will contain configuration necessary to launch instances in AWS. properties: amiFamily: description: AMIFamily is the AMI family that instances use. @@ -47,9 +52,11 @@ spec: description: AMISelector discovers AMIs to be used by Amazon EC2 tags. type: object apiVersion: - description: 'APIVersion defines the versioned schema of this representation - of an object. Servers should convert recognized schemas to the latest - internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources type: string blockDeviceMappings: description: BlockDeviceMappings to be applied to provisioned nodes. @@ -67,27 +74,38 @@ spec: volume is deleted on instance termination. type: boolean encrypted: - description: Encrypted indicates whether the EBS volume - is encrypted. Encrypted volumes can only be attached to - instances that support Amazon EBS encryption. If you are - creating a volume from a snapshot, you can't specify an - encryption value. + description: |- + Encrypted indicates whether the EBS volume is encrypted. Encrypted volumes can only + be attached to instances that support Amazon EBS encryption. If you are creating + a volume from a snapshot, you can't specify an encryption value. type: boolean iops: - description: "IOPS is the number of I/O operations per second - (IOPS). For gp3, io1, and io2 volumes, this represents - the number of IOPS that are provisioned for the volume. - For gp2 volumes, this represents the baseline performance - of the volume and the rate at which the volume accumulates - I/O credits for bursting. \n The following are the supported - values for each volume type: \n * gp3: 3,000-16,000 IOPS - \n * io1: 100-64,000 IOPS \n * io2: 100-64,000 IOPS \n - For io1 and io2 volumes, we guarantee 64,000 IOPS only - for Instances built on the Nitro System (https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/instance-types.html#ec2-nitro-instances). - Other instance families guarantee performance up to 32,000 - IOPS. \n This parameter is supported for io1, io2, and - gp3 volumes only. This parameter is not supported for - gp2, st1, sc1, or standard volumes." + description: |- + IOPS is the number of I/O operations per second (IOPS). For gp3, io1, and io2 volumes, + this represents the number of IOPS that are provisioned for the volume. For + gp2 volumes, this represents the baseline performance of the volume and the + rate at which the volume accumulates I/O credits for bursting. + + + The following are the supported values for each volume type: + + + * gp3: 3,000-16,000 IOPS + + + * io1: 100-64,000 IOPS + + + * io2: 100-64,000 IOPS + + + For io1 and io2 volumes, we guarantee 64,000 IOPS only for Instances built + on the Nitro System (https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/instance-types.html#ec2-nitro-instances). + Other instance families guarantee performance up to 32,000 IOPS. + + + This parameter is supported for io1, io2, and gp3 volumes only. This parameter + is not supported for gp2, st1, sc1, or standard volumes. format: int64 type: integer kmsKeyID: @@ -98,32 +116,46 @@ spec: description: SnapshotID is the ID of an EBS snapshot type: string throughput: - description: 'Throughput to provision for a gp3 volume, - with a maximum of 1,000 MiB/s. Valid Range: Minimum value - of 125. Maximum value of 1000.' + description: |- + Throughput to provision for a gp3 volume, with a maximum of 1,000 MiB/s. + Valid Range: Minimum value of 125. Maximum value of 1000. format: int64 type: integer volumeSize: anyOf: - type: integer - type: string - description: "VolumeSize in GiBs. You must specify either - a snapshot ID or a volume size. The following are the - supported volumes sizes for each volume type: \n * gp2 - and gp3: 1-16,384 \n * io1 and io2: 4-16,384 \n * st1 - and sc1: 125-16,384 \n * standard: 1-1,024" + description: |- + VolumeSize in GiBs. You must specify either a snapshot ID or + a volume size. The following are the supported volumes sizes for each volume + type: + + + * gp2 and gp3: 1-16,384 + + + * io1 and io2: 4-16,384 + + + * st1 and sc1: 125-16,384 + + + * standard: 1-1,024 pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ x-kubernetes-int-or-string: true volumeType: - description: VolumeType of the block device. For more information, - see Amazon EBS volume types (https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/EBSVolumeTypes.html) + description: |- + VolumeType of the block device. + For more information, see Amazon EBS volume types (https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/EBSVolumeTypes.html) in the Amazon Elastic Compute Cloud User Guide. type: string type: object type: object type: array context: - description: Context is a Reserved field in EC2 APIs https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_CreateFleet.html + description: |- + Context is a Reserved field in EC2 APIs + https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_CreateFleet.html type: string detailedMonitoring: description: DetailedMonitoring controls if detailed monitoring is @@ -133,63 +165,81 @@ spec: description: InstanceProfile is the AWS identity that instances use. type: string kind: - description: 'Kind is a string value representing the REST resource - this object represents. Servers may infer this from the endpoint - the client submits requests to. Cannot be updated. In CamelCase. - More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds type: string launchTemplate: - description: 'LaunchTemplateName for the node. If not specified, a - launch template will be generated. NOTE: This field is for specifying - a custom launch template and is exposed in the Spec as `launchTemplate` - for backwards compatibility.' + description: |- + LaunchTemplateName for the node. If not specified, a launch template will be generated. + NOTE: This field is for specifying a custom launch template and is exposed in the Spec + as `launchTemplate` for backwards compatibility. type: string metadataOptions: - description: "MetadataOptions for the generated launch template of - provisioned nodes. \n This specifies the exposure of the Instance - Metadata Service to provisioned EC2 nodes. For more information, - see Instance Metadata and User Data (https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/ec2-instance-metadata.html) - in the Amazon Elastic Compute Cloud User Guide. \n Refer to recommended, - security best practices (https://aws.github.io/aws-eks-best-practices/security/docs/iam/#restrict-access-to-the-instance-profile-assigned-to-the-worker-node) + description: |- + MetadataOptions for the generated launch template of provisioned nodes. + + + This specifies the exposure of the Instance Metadata Service to + provisioned EC2 nodes. For more information, + see Instance Metadata and User Data + (https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/ec2-instance-metadata.html) + in the Amazon Elastic Compute Cloud User Guide. + + + Refer to recommended, security best practices + (https://aws.github.io/aws-eks-best-practices/security/docs/iam/#restrict-access-to-the-instance-profile-assigned-to-the-worker-node) for limiting exposure of Instance Metadata and User Data to pods. If omitted, defaults to httpEndpoint enabled, with httpProtocolIPv6 - disabled, with httpPutResponseLimit of 2, and with httpTokens required." + disabled, with httpPutResponseLimit of 2, and with httpTokens + required. properties: httpEndpoint: - description: "HTTPEndpoint enables or disables the HTTP metadata - endpoint on provisioned nodes. If metadata options is non-nil, - but this parameter is not specified, the default state is \"enabled\". - \n If you specify a value of \"disabled\", instance metadata - will not be accessible on the node." + description: |- + HTTPEndpoint enables or disables the HTTP metadata endpoint on provisioned + nodes. If metadata options is non-nil, but this parameter is not specified, + the default state is "enabled". + + + If you specify a value of "disabled", instance metadata will not be accessible + on the node. type: string httpProtocolIPv6: - description: HTTPProtocolIPv6 enables or disables the IPv6 endpoint - for the instance metadata service on provisioned nodes. If metadata - options is non-nil, but this parameter is not specified, the - default state is "disabled". + description: |- + HTTPProtocolIPv6 enables or disables the IPv6 endpoint for the instance metadata + service on provisioned nodes. If metadata options is non-nil, but this parameter + is not specified, the default state is "disabled". type: string httpPutResponseHopLimit: - description: HTTPPutResponseHopLimit is the desired HTTP PUT response - hop limit for instance metadata requests. The larger the number, - the further instance metadata requests can travel. Possible - values are integers from 1 to 64. If metadata options is non-nil, - but this parameter is not specified, the default value is 1. + description: |- + HTTPPutResponseHopLimit is the desired HTTP PUT response hop limit for + instance metadata requests. The larger the number, the further instance + metadata requests can travel. Possible values are integers from 1 to 64. + If metadata options is non-nil, but this parameter is not specified, the + default value is 1. format: int64 type: integer httpTokens: - description: "HTTPTokens determines the state of token usage for - instance metadata requests. If metadata options is non-nil, - but this parameter is not specified, the default state is \"optional\". - \n If the state is optional, one can choose to retrieve instance - metadata with or without a signed token header on the request. - If one retrieves the IAM role credentials without a token, the - version 1.0 role credentials are returned. If one retrieves - the IAM role credentials using a valid signed token, the version - 2.0 role credentials are returned. \n If the state is \"required\", - one must send a signed token header with any instance metadata - retrieval requests. In this state, retrieving the IAM role credentials - always returns the version 2.0 credentials; the version 1.0 - credentials are not available." + description: |- + HTTPTokens determines the state of token usage for instance metadata + requests. If metadata options is non-nil, but this parameter is not + specified, the default state is "optional". + + + If the state is optional, one can choose to retrieve instance metadata with + or without a signed token header on the request. If one retrieves the IAM + role credentials without a token, the version 1.0 role credentials are + returned. If one retrieves the IAM role credentials using a valid signed + token, the version 2.0 role credentials are returned. + + + If the state is "required", one must send a signed token header with any + instance metadata retrieval requests. In this state, retrieving the IAM + role credentials always returns the version 2.0 credentials; the version + 1.0 credentials are not available. type: string type: object securityGroupSelector: @@ -210,10 +260,10 @@ spec: launch templates. type: object userData: - description: UserData to be applied to the provisioned nodes. It must - be in the appropriate format based on the AMIFamily in use. Karpenter - will merge certain fields into this UserData to ensure nodes are - being provisioned with the correct configuration. + description: |- + UserData to be applied to the provisioned nodes. + It must be in the appropriate format based on the AMIFamily in use. Karpenter will merge certain fields into + this UserData to ensure nodes are being provisioned with the correct configuration. type: string type: object status: @@ -221,8 +271,9 @@ spec: AWSNodeTemplate properties: amis: - description: AMI contains the current AMI values that are available - to the cluster under the AMI selectors. + description: |- + AMI contains the current AMI values that are available to the + cluster under the AMI selectors. items: description: AMI contains resolved AMI selector values utilized for node launch @@ -237,26 +288,25 @@ spec: description: Requirements of the AMI to be utilized on an instance type items: - description: A node selector requirement is a selector that - contains values, a key, and an operator that relates the - key and values. + description: |- + A node selector requirement is a selector that contains values, a key, and an operator + that relates the key and values. properties: key: description: The label key that the selector applies to. type: string operator: - description: Represents a key's relationship to a set - of values. Valid operators are In, NotIn, Exists, DoesNotExist. - Gt, and Lt. + description: |- + Represents a key's relationship to a set of values. + Valid operators are In, NotIn, Exists, DoesNotExist. Gt, and Lt. type: string values: - description: An array of string values. If the operator - is In or NotIn, the values array must be non-empty. - If the operator is Exists or DoesNotExist, the values - array must be empty. If the operator is Gt or Lt, the - values array must have a single element, which will - be interpreted as an integer. This array is replaced - during a strategic merge patch. + description: |- + An array of string values. If the operator is In or NotIn, + the values array must be non-empty. If the operator is Exists or DoesNotExist, + the values array must be empty. If the operator is Gt or Lt, the values + array must have a single element, which will be interpreted as an integer. + This array is replaced during a strategic merge patch. items: type: string type: array @@ -271,8 +321,9 @@ spec: type: object type: array securityGroups: - description: SecurityGroups contains the current Security Groups values - that are available to the cluster under the SecurityGroups selectors. + description: |- + SecurityGroups contains the current Security Groups values that are available to the + cluster under the SecurityGroups selectors. items: description: SecurityGroup contains resolved SecurityGroup selector values utilized for node launch @@ -288,8 +339,9 @@ spec: type: object type: array subnets: - description: Subnets contains the current Subnet values that are available - to the cluster under the subnet selectors. + description: |- + Subnets contains the current Subnet values that are available to the + cluster under the subnet selectors. items: description: Subnet contains resolved Subnet selector values utilized for node launch diff --git a/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml b/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml index 467ff450d4f2..329aba7fba0b 100644 --- a/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml +++ b/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml @@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.13.0 + controller-gen.kubebuilder.io/version: v0.15.0 name: ec2nodeclasses.karpenter.k8s.aws spec: group: karpenter.k8s.aws @@ -25,21 +25,26 @@ spec: description: EC2NodeClass is the Schema for the EC2NodeClass API properties: apiVersion: - description: 'APIVersion defines the versioned schema of this representation - of an object. Servers should convert recognized schemas to the latest - internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources type: string kind: - description: 'Kind is a string value representing the REST resource this - object represents. Servers may infer this from the endpoint the client - submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds type: string metadata: type: object spec: - description: EC2NodeClassSpec is the top level specification for the AWS - Karpenter Provider. This will contain configuration necessary to launch - instances in AWS. + description: |- + EC2NodeClassSpec is the top level specification for the AWS Karpenter Provider. + This will contain configuration necessary to launch instances in AWS. properties: amiFamily: description: AMIFamily is the AMI family that instances use. @@ -55,28 +60,30 @@ spec: description: AMISelectorTerms is a list of or ami selector terms. The terms are ORed. items: - description: AMISelectorTerm defines selection logic for an ami - used by Karpenter to launch nodes. If multiple fields are used - for selection, the requirements are ANDed. + description: |- + AMISelectorTerm defines selection logic for an ami used by Karpenter to launch nodes. + If multiple fields are used for selection, the requirements are ANDed. properties: id: description: ID is the ami id in EC2 pattern: ami-[0-9a-z]+ type: string name: - description: Name is the ami name in EC2. This value is the - name field, which is different from the name tag. + description: |- + Name is the ami name in EC2. + This value is the name field, which is different from the name tag. type: string owner: - description: Owner is the owner for the ami. You can specify - a combination of AWS account IDs, "self", "amazon", and "aws-marketplace" + description: |- + Owner is the owner for the ami. + You can specify a combination of AWS account IDs, "self", "amazon", and "aws-marketplace" type: string tags: additionalProperties: type: string - description: Tags is a map of key/value tags used to select - subnets Specifying '*' for a value selects all values for - a given tag key. + description: |- + Tags is a map of key/value tags used to select subnets + Specifying '*' for a value selects all values for a given tag key. maxProperties: 20 type: object x-kubernetes-validations: @@ -108,27 +115,38 @@ spec: volume is deleted on instance termination. type: boolean encrypted: - description: Encrypted indicates whether the EBS volume - is encrypted. Encrypted volumes can only be attached to - instances that support Amazon EBS encryption. If you are - creating a volume from a snapshot, you can't specify an - encryption value. + description: |- + Encrypted indicates whether the EBS volume is encrypted. Encrypted volumes can only + be attached to instances that support Amazon EBS encryption. If you are creating + a volume from a snapshot, you can't specify an encryption value. type: boolean iops: - description: "IOPS is the number of I/O operations per second - (IOPS). For gp3, io1, and io2 volumes, this represents - the number of IOPS that are provisioned for the volume. - For gp2 volumes, this represents the baseline performance - of the volume and the rate at which the volume accumulates - I/O credits for bursting. \n The following are the supported - values for each volume type: \n * gp3: 3,000-16,000 IOPS - \n * io1: 100-64,000 IOPS \n * io2: 100-64,000 IOPS \n - For io1 and io2 volumes, we guarantee 64,000 IOPS only - for Instances built on the Nitro System (https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/instance-types.html#ec2-nitro-instances). - Other instance families guarantee performance up to 32,000 - IOPS. \n This parameter is supported for io1, io2, and - gp3 volumes only. This parameter is not supported for - gp2, st1, sc1, or standard volumes." + description: |- + IOPS is the number of I/O operations per second (IOPS). For gp3, io1, and io2 volumes, + this represents the number of IOPS that are provisioned for the volume. For + gp2 volumes, this represents the baseline performance of the volume and the + rate at which the volume accumulates I/O credits for bursting. + + + The following are the supported values for each volume type: + + + * gp3: 3,000-16,000 IOPS + + + * io1: 100-64,000 IOPS + + + * io2: 100-64,000 IOPS + + + For io1 and io2 volumes, we guarantee 64,000 IOPS only for Instances built + on the Nitro System (https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/instance-types.html#ec2-nitro-instances). + Other instance families guarantee performance up to 32,000 IOPS. + + + This parameter is supported for io1, io2, and gp3 volumes only. This parameter + is not supported for gp2, st1, sc1, or standard volumes. format: int64 type: integer kmsKeyID: @@ -139,9 +157,9 @@ spec: description: SnapshotID is the ID of an EBS snapshot type: string throughput: - description: 'Throughput to provision for a gp3 volume, - with a maximum of 1,000 MiB/s. Valid Range: Minimum value - of 125. Maximum value of 1000.' + description: |- + Throughput to provision for a gp3 volume, with a maximum of 1,000 MiB/s. + Valid Range: Minimum value of 125. Maximum value of 1000. format: int64 type: integer volumeSize: @@ -151,15 +169,27 @@ spec: anyOf: - type: integer - type: string - description: "VolumeSize in `Gi`, `G`, `Ti`, or `T`. You - must specify either a snapshot ID or a volume size. The - following are the supported volumes sizes for each volume - type: \n * gp2 and gp3: 1-16,384 \n * io1 and io2: 4-16,384 - \n * st1 and sc1: 125-16,384 \n * standard: 1-1,024" + description: |- + VolumeSize in `Gi`, `G`, `Ti`, or `T`. You must specify either a snapshot ID or + a volume size. The following are the supported volumes sizes for each volume + type: + + + * gp2 and gp3: 1-16,384 + + + * io1 and io2: 4-16,384 + + + * st1 and sc1: 125-16,384 + + + * standard: 1-1,024 x-kubernetes-int-or-string: true volumeType: - description: VolumeType of the block device. For more information, - see Amazon EBS volume types (https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/EBSVolumeTypes.html) + description: |- + VolumeType of the block device. + For more information, see Amazon EBS volume types (https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/EBSVolumeTypes.html) in the Amazon Elastic Compute Cloud User Guide. enum: - standard @@ -175,9 +205,9 @@ spec: - message: snapshotID or volumeSize must be defined rule: has(self.snapshotID) || has(self.volumeSize) rootVolume: - description: RootVolume is a flag indicating if this device - is mounted as kubelet root dir. You can configure at most - one root volume in BlockDeviceMappings. + description: |- + RootVolume is a flag indicating if this device is mounted as kubelet root dir. You can + configure at most one root volume in BlockDeviceMappings. type: boolean type: object maxItems: 50 @@ -187,18 +217,20 @@ spec: rule: self.filter(x, has(x.rootVolume)?x.rootVolume==true:false).size() <= 1 context: - description: Context is a Reserved field in EC2 APIs https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_CreateFleet.html + description: |- + Context is a Reserved field in EC2 APIs + https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_CreateFleet.html type: string detailedMonitoring: description: DetailedMonitoring controls if detailed monitoring is enabled for instances that are launched type: boolean instanceProfile: - description: InstanceProfile is the AWS entity that instances use. - This field is mutually exclusive from role. The instance profile - should already have a role assigned to it that Karpenter has PassRole - permission on for instance launch using this instanceProfile to - succeed. + description: |- + InstanceProfile is the AWS entity that instances use. + This field is mutually exclusive from role. + The instance profile should already have a role assigned to it that Karpenter + has PassRole permission on for instance launch using this instanceProfile to succeed. type: string x-kubernetes-validations: - message: instanceProfile cannot be empty @@ -209,76 +241,91 @@ spec: httpProtocolIPv6: disabled httpPutResponseHopLimit: 2 httpTokens: required - description: "MetadataOptions for the generated launch template of - provisioned nodes. \n This specifies the exposure of the Instance - Metadata Service to provisioned EC2 nodes. For more information, - see Instance Metadata and User Data (https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/ec2-instance-metadata.html) - in the Amazon Elastic Compute Cloud User Guide. \n Refer to recommended, - security best practices (https://aws.github.io/aws-eks-best-practices/security/docs/iam/#restrict-access-to-the-instance-profile-assigned-to-the-worker-node) + description: |- + MetadataOptions for the generated launch template of provisioned nodes. + + + This specifies the exposure of the Instance Metadata Service to + provisioned EC2 nodes. For more information, + see Instance Metadata and User Data + (https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/ec2-instance-metadata.html) + in the Amazon Elastic Compute Cloud User Guide. + + + Refer to recommended, security best practices + (https://aws.github.io/aws-eks-best-practices/security/docs/iam/#restrict-access-to-the-instance-profile-assigned-to-the-worker-node) for limiting exposure of Instance Metadata and User Data to pods. If omitted, defaults to httpEndpoint enabled, with httpProtocolIPv6 - disabled, with httpPutResponseLimit of 2, and with httpTokens required." + disabled, with httpPutResponseLimit of 2, and with httpTokens + required. properties: httpEndpoint: default: enabled - description: "HTTPEndpoint enables or disables the HTTP metadata - endpoint on provisioned nodes. If metadata options is non-nil, - but this parameter is not specified, the default state is \"enabled\". - \n If you specify a value of \"disabled\", instance metadata - will not be accessible on the node." + description: |- + HTTPEndpoint enables or disables the HTTP metadata endpoint on provisioned + nodes. If metadata options is non-nil, but this parameter is not specified, + the default state is "enabled". + + + If you specify a value of "disabled", instance metadata will not be accessible + on the node. enum: - enabled - disabled type: string httpProtocolIPv6: default: disabled - description: HTTPProtocolIPv6 enables or disables the IPv6 endpoint - for the instance metadata service on provisioned nodes. If metadata - options is non-nil, but this parameter is not specified, the - default state is "disabled". + description: |- + HTTPProtocolIPv6 enables or disables the IPv6 endpoint for the instance metadata + service on provisioned nodes. If metadata options is non-nil, but this parameter + is not specified, the default state is "disabled". enum: - enabled - disabled type: string httpPutResponseHopLimit: default: 2 - description: HTTPPutResponseHopLimit is the desired HTTP PUT response - hop limit for instance metadata requests. The larger the number, - the further instance metadata requests can travel. Possible - values are integers from 1 to 64. If metadata options is non-nil, - but this parameter is not specified, the default value is 2. + description: |- + HTTPPutResponseHopLimit is the desired HTTP PUT response hop limit for + instance metadata requests. The larger the number, the further instance + metadata requests can travel. Possible values are integers from 1 to 64. + If metadata options is non-nil, but this parameter is not specified, the + default value is 2. format: int64 maximum: 64 minimum: 1 type: integer httpTokens: default: required - description: "HTTPTokens determines the state of token usage for - instance metadata requests. If metadata options is non-nil, - but this parameter is not specified, the default state is \"required\". - \n If the state is optional, one can choose to retrieve instance - metadata with or without a signed token header on the request. - If one retrieves the IAM role credentials without a token, the - version 1.0 role credentials are returned. If one retrieves - the IAM role credentials using a valid signed token, the version - 2.0 role credentials are returned. \n If the state is \"required\", - one must send a signed token header with any instance metadata - retrieval requests. In this state, retrieving the IAM role credentials - always returns the version 2.0 credentials; the version 1.0 - credentials are not available." + description: |- + HTTPTokens determines the state of token usage for instance metadata + requests. If metadata options is non-nil, but this parameter is not + specified, the default state is "required". + + + If the state is optional, one can choose to retrieve instance metadata with + or without a signed token header on the request. If one retrieves the IAM + role credentials without a token, the version 1.0 role credentials are + returned. If one retrieves the IAM role credentials using a valid signed + token, the version 2.0 role credentials are returned. + + + If the state is "required", one must send a signed token header with any + instance metadata retrieval requests. In this state, retrieving the IAM + role credentials always returns the version 2.0 credentials; the version + 1.0 credentials are not available. enum: - required - optional type: string type: object role: - description: Role is the AWS identity that nodes use. This field is - immutable. This field is mutually exclusive from instanceProfile. - Marking this field as immutable avoids concerns around terminating - managed instance profiles from running instances. This field may - be made mutable in the future, assuming the correct garbage collection - and drift handling is implemented for the old instance profiles - on an update. + description: |- + Role is the AWS identity that nodes use. This field is immutable. + This field is mutually exclusive from instanceProfile. + Marking this field as immutable avoids concerns around terminating managed instance profiles from running instances. + This field may be made mutable in the future, assuming the correct garbage collection and drift handling is implemented + for the old instance profiles on an update. type: string x-kubernetes-validations: - message: role cannot be empty @@ -289,24 +336,25 @@ spec: description: SecurityGroupSelectorTerms is a list of or security group selector terms. The terms are ORed. items: - description: SecurityGroupSelectorTerm defines selection logic for - a security group used by Karpenter to launch nodes. If multiple - fields are used for selection, the requirements are ANDed. + description: |- + SecurityGroupSelectorTerm defines selection logic for a security group used by Karpenter to launch nodes. + If multiple fields are used for selection, the requirements are ANDed. properties: id: description: ID is the security group id in EC2 pattern: sg-[0-9a-z]+ type: string name: - description: Name is the security group name in EC2. This value - is the name field, which is different from the name tag. + description: |- + Name is the security group name in EC2. + This value is the name field, which is different from the name tag. type: string tags: additionalProperties: type: string - description: Tags is a map of key/value tags used to select - subnets Specifying '*' for a value selects all values for - a given tag key. + description: |- + Tags is a map of key/value tags used to select subnets + Specifying '*' for a value selects all values for a given tag key. maxProperties: 20 type: object x-kubernetes-validations: @@ -330,9 +378,9 @@ spec: description: SubnetSelectorTerms is a list of or subnet selector terms. The terms are ORed. items: - description: SubnetSelectorTerm defines selection logic for a subnet - used by Karpenter to launch nodes. If multiple fields are used - for selection, the requirements are ANDed. + description: |- + SubnetSelectorTerm defines selection logic for a subnet used by Karpenter to launch nodes. + If multiple fields are used for selection, the requirements are ANDed. properties: id: description: ID is the subnet id in EC2 @@ -341,9 +389,9 @@ spec: tags: additionalProperties: type: string - description: Tags is a map of key/value tags used to select - subnets Specifying '*' for a value selects all values for - a given tag key. + description: |- + Tags is a map of key/value tags used to select subnets + Specifying '*' for a value selects all values for a given tag key. maxProperties: 20 type: object x-kubernetes-validations: @@ -378,10 +426,10 @@ spec: - message: tag contains a restricted tag matching karpenter.sh/managed-by rule: self.all(k, k !='karpenter.sh/managed-by') userData: - description: UserData to be applied to the provisioned nodes. It must - be in the appropriate format based on the AMIFamily in use. Karpenter - will merge certain fields into this UserData to ensure nodes are - being provisioned with the correct configuration. + description: |- + UserData to be applied to the provisioned nodes. + It must be in the appropriate format based on the AMIFamily in use. Karpenter will merge certain fields into + this UserData to ensure nodes are being provisioned with the correct configuration. type: string required: - amiFamily @@ -404,8 +452,9 @@ spec: description: EC2NodeClassStatus contains the resolved state of the EC2NodeClass properties: amis: - description: AMI contains the current AMI values that are available - to the cluster under the AMI selectors. + description: |- + AMI contains the current AMI values that are available to the + cluster under the AMI selectors. items: description: AMI contains resolved AMI selector values utilized for node launch @@ -420,26 +469,25 @@ spec: description: Requirements of the AMI to be utilized on an instance type items: - description: A node selector requirement is a selector that - contains values, a key, and an operator that relates the - key and values. + description: |- + A node selector requirement is a selector that contains values, a key, and an operator + that relates the key and values. properties: key: description: The label key that the selector applies to. type: string operator: - description: Represents a key's relationship to a set - of values. Valid operators are In, NotIn, Exists, DoesNotExist. - Gt, and Lt. + description: |- + Represents a key's relationship to a set of values. + Valid operators are In, NotIn, Exists, DoesNotExist. Gt, and Lt. type: string values: - description: An array of string values. If the operator - is In or NotIn, the values array must be non-empty. - If the operator is Exists or DoesNotExist, the values - array must be empty. If the operator is Gt or Lt, the - values array must have a single element, which will - be interpreted as an integer. This array is replaced - during a strategic merge patch. + description: |- + An array of string values. If the operator is In or NotIn, + the values array must be non-empty. If the operator is Exists or DoesNotExist, + the values array must be empty. If the operator is Gt or Lt, the values + array must have a single element, which will be interpreted as an integer. + This array is replaced during a strategic merge patch. items: type: string type: array @@ -458,8 +506,9 @@ spec: for the role type: string securityGroups: - description: SecurityGroups contains the current Security Groups values - that are available to the cluster under the SecurityGroups selectors. + description: |- + SecurityGroups contains the current Security Groups values that are available to the + cluster under the SecurityGroups selectors. items: description: SecurityGroup contains resolved SecurityGroup selector values utilized for node launch @@ -475,8 +524,9 @@ spec: type: object type: array subnets: - description: Subnets contains the current Subnet values that are available - to the cluster under the subnet selectors. + description: |- + Subnets contains the current Subnet values that are available to the + cluster under the subnet selectors. items: description: Subnet contains resolved Subnet selector values utilized for node launch diff --git a/pkg/providers/amifamily/ami.go b/pkg/providers/amifamily/ami.go index bc49d811b5ef..f62552b74ca9 100644 --- a/pkg/providers/amifamily/ami.go +++ b/pkg/providers/amifamily/ami.go @@ -138,7 +138,9 @@ func (p *Provider) Get(ctx context.Context, nodeClass *v1beta1.EC2NodeClass, opt func (p *Provider) getDefaultAMIs(ctx context.Context, nodeClass *v1beta1.EC2NodeClass, options *Options) (res AMIs, err error) { if images, ok := p.cache.Get(lo.FromPtr(nodeClass.Spec.AMIFamily)); ok { - return images.(AMIs), nil + // Ensure what's returned from this function is a deep-copy of AMIs so alterations + // to the data don't affect the original + return append(AMIs{}, images.(AMIs)...), nil } amiFamily := GetAMIFamily(nodeClass.Spec.AMIFamily, options) kubernetesVersion, err := p.versionProvider.Get(ctx) @@ -189,8 +191,10 @@ func (p *Provider) getAMIs(ctx context.Context, terms []v1beta1.AMISelectorTerm, if err != nil { return nil, err } - if images, ok := p.cache.Get(fmt.Sprintf("%t/%d", isNodeTemplate, hash)); ok { - return images.(AMIs), nil + if images, ok := p.cache.Get(fmt.Sprintf("%d", hash)); ok { + // Ensure what's returned from this function is a deep-copy of AMIs so alterations + // to the data don't affect the original + return append(AMIs{}, images.(AMIs)...), nil } images := map[uint64]AMI{} for _, filtersAndOwners := range filterAndOwnerSets { diff --git a/pkg/providers/amifamily/ami_test.go b/pkg/providers/amifamily/ami_test.go index 823682c4d516..dc236efc8cf9 100644 --- a/pkg/providers/amifamily/ami_test.go +++ b/pkg/providers/amifamily/ami_test.go @@ -18,6 +18,7 @@ import ( "context" "fmt" "sort" + "sync" "testing" "time" @@ -30,6 +31,7 @@ import ( . "knative.dev/pkg/logging/testing" "github.com/aws/karpenter-core/pkg/apis/v1alpha5" + corev1beta1 "github.com/aws/karpenter-core/pkg/apis/v1beta1" coreoptions "github.com/aws/karpenter-core/pkg/operator/options" "github.com/aws/karpenter-core/pkg/operator/scheme" "github.com/aws/karpenter-core/pkg/scheduling" @@ -75,7 +77,7 @@ var _ = BeforeEach(func() { { Name: aws.String(amd64AMI), ImageId: aws.String("amd64-ami-id"), - CreationDate: aws.String(time.Now().Format(time.RFC3339)), + CreationDate: aws.String(time.Time{}.Format(time.RFC3339)), Architecture: aws.String("x86_64"), Tags: []*ec2.Tag{ {Key: aws.String("Name"), Value: aws.String(amd64AMI)}, @@ -85,7 +87,7 @@ var _ = BeforeEach(func() { { Name: aws.String(arm64AMI), ImageId: aws.String("arm64-ami-id"), - CreationDate: aws.String(time.Now().Add(time.Minute).Format(time.RFC3339)), + CreationDate: aws.String(time.Time{}.Add(time.Minute).Format(time.RFC3339)), Architecture: aws.String("arm64"), Tags: []*ec2.Tag{ {Key: aws.String("Name"), Value: aws.String(arm64AMI)}, @@ -95,7 +97,7 @@ var _ = BeforeEach(func() { { Name: aws.String(amd64NvidiaAMI), ImageId: aws.String("amd64-nvidia-ami-id"), - CreationDate: aws.String(time.Now().Add(2 * time.Minute).Format(time.RFC3339)), + CreationDate: aws.String(time.Time{}.Add(2 * time.Minute).Format(time.RFC3339)), Architecture: aws.String("x86_64"), Tags: []*ec2.Tag{ {Key: aws.String("Name"), Value: aws.String(amd64NvidiaAMI)}, @@ -105,7 +107,7 @@ var _ = BeforeEach(func() { { Name: aws.String(arm64NvidiaAMI), ImageId: aws.String("arm64-nvidia-ami-id"), - CreationDate: aws.String(time.Now().Add(2 * time.Minute).Format(time.RFC3339)), + CreationDate: aws.String(time.Time{}.Add(2 * time.Minute).Format(time.RFC3339)), Architecture: aws.String("arm64"), Tags: []*ec2.Tag{ {Key: aws.String("Name"), Value: aws.String(arm64NvidiaAMI)}, @@ -187,6 +189,49 @@ var _ = Describe("AMIProvider", func() { Expect(err).ToNot(HaveOccurred()) Expect(amis).To(HaveLen(0)) }) + It("should not cause data races when calling Get() simultaneously", func() { + nodeClass.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ + { + ID: "amd64-ami-id", + }, + { + ID: "arm64-ami-id", + }, + } + wg := sync.WaitGroup{} + for i := 0; i < 10000; i++ { + wg.Add(1) + go func() { + defer wg.Done() + defer GinkgoRecover() + images, err := awsEnv.AMIProvider.Get(ctx, nodeClass, &amifamily.Options{}) + Expect(err).ToNot(HaveOccurred()) + + Expect(images).To(HaveLen(2)) + // Sort everything in parallel and ensure that we don't get data races + images.Sort() + Expect(images).To(BeEquivalentTo([]amifamily.AMI{ + { + Name: arm64AMI, + AmiID: "arm64-ami-id", + CreationDate: time.Time{}.Add(time.Minute).Format(time.RFC3339), + Requirements: scheduling.NewLabelRequirements(map[string]string{ + v1.LabelArchStable: corev1beta1.ArchitectureArm64, + }), + }, + { + Name: amd64AMI, + AmiID: "amd64-ami-id", + CreationDate: time.Time{}.Format(time.RFC3339), + Requirements: scheduling.NewLabelRequirements(map[string]string{ + v1.LabelArchStable: corev1beta1.ArchitectureAmd64, + }), + }, + })) + }() + } + wg.Wait() + }) Context("SSM Alias Missing", func() { It("should succeed to partially resolve AMIs if all SSM aliases don't exist (Al2)", func() { nodeClass.Spec.AMIFamily = &v1beta1.AMIFamilyAL2 diff --git a/pkg/providers/instancetype/instancetype.go b/pkg/providers/instancetype/instancetype.go index 5766a5b7f84a..4fbbe056875e 100644 --- a/pkg/providers/instancetype/instancetype.go +++ b/pkg/providers/instancetype/instancetype.go @@ -139,7 +139,9 @@ func (p *Provider) List(ctx context.Context, kc *corev1beta1.KubeletConfiguratio systemReservedHash, ) if item, ok := p.cache.Get(key); ok { - return item.([]*cloudprovider.InstanceType), nil + // Ensure what's returned from this function is a shallow-copy of the slice (not a deep-copy of the data itself) + // so that modifications to the ordering of the data don't affect the original + return append([]*cloudprovider.InstanceType{}, item.([]*cloudprovider.InstanceType)...), nil } result := lo.Map(instanceTypes, func(i *ec2.InstanceTypeInfo, _ int) *cloudprovider.InstanceType { return NewInstanceType(ctx, i, kc, p.region, nodeClass, p.createOfferings(ctx, i, instanceTypeOfferings[aws.StringValue(i.InstanceType)], zones, subnetZones)) diff --git a/pkg/providers/instancetype/nodeclass_test.go b/pkg/providers/instancetype/nodeclass_test.go index 620a01e03149..0e50d70bba95 100644 --- a/pkg/providers/instancetype/nodeclass_test.go +++ b/pkg/providers/instancetype/nodeclass_test.go @@ -19,6 +19,7 @@ import ( "math" "sort" "strings" + "sync" "time" "github.com/aws/aws-sdk-go/aws" @@ -1493,4 +1494,39 @@ var _ = Describe("NodeClass/InstanceTypes", func() { }) }) }) + It("should not cause data races when calling List() simultaneously", func() { + mu := sync.RWMutex{} + var instanceTypeOrder []string + wg := sync.WaitGroup{} + for i := 0; i < 10000; i++ { + wg.Add(1) + go func() { + defer wg.Done() + defer GinkgoRecover() + instanceTypes, err := awsEnv.InstanceTypesProvider.List(ctx, &corev1beta1.KubeletConfiguration{}, &v1beta1.EC2NodeClass{}) + Expect(err).ToNot(HaveOccurred()) + + // Sort everything in parallel and ensure that we don't get data races + sort.Slice(instanceTypes, func(i, j int) bool { + return instanceTypes[i].Name < instanceTypes[j].Name + }) + // Get the ordering of the instance types based on name + tempInstanceTypeOrder := lo.Map(instanceTypes, func(i *corecloudprovider.InstanceType, _ int) string { + return i.Name + }) + // Expect that all the elements in the instance type list are unique + Expect(lo.Uniq(tempInstanceTypeOrder)).To(HaveLen(len(tempInstanceTypeOrder))) + + // We have to lock since we are doing simultaneous access to this value + mu.Lock() + if len(instanceTypeOrder) == 0 { + instanceTypeOrder = tempInstanceTypeOrder + } else { + Expect(tempInstanceTypeOrder).To(BeEquivalentTo(instanceTypeOrder)) + } + mu.Unlock() + }() + } + wg.Wait() + }) }) diff --git a/pkg/providers/instancetype/nodetemplate_test.go b/pkg/providers/instancetype/nodetemplate_test.go index 0edb10700d77..0f2afbd7ca75 100644 --- a/pkg/providers/instancetype/nodetemplate_test.go +++ b/pkg/providers/instancetype/nodetemplate_test.go @@ -19,6 +19,7 @@ import ( "math" "sort" "strings" + "sync" "time" "github.com/aws/aws-sdk-go/aws" @@ -1523,4 +1524,39 @@ var _ = Describe("NodeTemplate/InstanceTypes", func() { }) }) }) + It("should not cause data races when calling List() simultaneously", func() { + mu := sync.RWMutex{} + var instanceTypeOrder []string + wg := sync.WaitGroup{} + for i := 0; i < 10000; i++ { + wg.Add(1) + go func() { + defer wg.Done() + defer GinkgoRecover() + instanceTypes, err := awsEnv.InstanceTypesProvider.List(ctx, nodepoolutil.NewKubeletConfiguration(&v1alpha5.KubeletConfiguration{}), nodeclassutil.New(&v1alpha1.AWSNodeTemplate{})) + Expect(err).ToNot(HaveOccurred()) + + // Sort everything in parallel and ensure that we don't get data races + sort.Slice(instanceTypes, func(i, j int) bool { + return instanceTypes[i].Name < instanceTypes[j].Name + }) + // Get the ordering of the instance types based on name + tempInstanceTypeOrder := lo.Map(instanceTypes, func(i *corecloudprovider.InstanceType, _ int) string { + return i.Name + }) + // Expect that all the elements in the instance type list are unique + Expect(lo.Uniq(tempInstanceTypeOrder)).To(HaveLen(len(tempInstanceTypeOrder))) + + // We have to lock since we are doing simultaneous access to this value + mu.Lock() + if len(instanceTypeOrder) == 0 { + instanceTypeOrder = tempInstanceTypeOrder + } else { + Expect(tempInstanceTypeOrder).To(BeEquivalentTo(instanceTypeOrder)) + } + mu.Unlock() + }() + } + wg.Wait() + }) }) diff --git a/pkg/providers/securitygroup/securitygroup.go b/pkg/providers/securitygroup/securitygroup.go index 18b1bef92289..56159392ebaf 100644 --- a/pkg/providers/securitygroup/securitygroup.go +++ b/pkg/providers/securitygroup/securitygroup.go @@ -81,7 +81,9 @@ func (p *Provider) getSecurityGroups(ctx context.Context, filterSets [][]*ec2.Fi return nil, err } if sg, ok := p.cache.Get(fmt.Sprint(hash)); ok { - return sg.([]*ec2.SecurityGroup), nil + // Ensure what's returned from this function is a shallow-copy of the slice (not a deep-copy of the data itself) + // so that modifications to the ordering of the data don't affect the original + return append([]*ec2.SecurityGroup{}, sg.([]*ec2.SecurityGroup)...), nil } securityGroups := map[string]*ec2.SecurityGroup{} for _, filters := range filterSets { diff --git a/pkg/providers/securitygroup/suite_test.go b/pkg/providers/securitygroup/suite_test.go index 6cd9bd8cf88e..740a30c68599 100644 --- a/pkg/providers/securitygroup/suite_test.go +++ b/pkg/providers/securitygroup/suite_test.go @@ -16,6 +16,8 @@ package securitygroup_test import ( "context" + "sort" + "sync" "testing" "github.com/aws/aws-sdk-go/aws" @@ -267,6 +269,72 @@ var _ = Describe("SecurityGroupProvider", func() { }, }, securityGroups) }) + It("should not cause data races when calling List() simultaneously", func() { + wg := sync.WaitGroup{} + for i := 0; i < 10000; i++ { + wg.Add(1) + go func() { + defer wg.Done() + defer GinkgoRecover() + securityGroups, err := awsEnv.SecurityGroupProvider.List(ctx, nodeClass) + Expect(err).ToNot(HaveOccurred()) + + Expect(securityGroups).To(HaveLen(3)) + // Sort everything in parallel and ensure that we don't get data races + sort.Slice(securityGroups, func(i, j int) bool { + return *securityGroups[i].GroupId < *securityGroups[j].GroupId + }) + Expect(securityGroups).To(BeEquivalentTo([]*ec2.SecurityGroup{ + { + GroupId: lo.ToPtr("sg-test1"), + GroupName: lo.ToPtr("securityGroup-test1"), + Tags: []*ec2.Tag{ + { + Key: lo.ToPtr("Name"), + Value: lo.ToPtr("test-security-group-1"), + }, + { + Key: lo.ToPtr("foo"), + Value: lo.ToPtr("bar"), + }, + }, + }, + { + GroupId: lo.ToPtr("sg-test2"), + GroupName: lo.ToPtr("securityGroup-test2"), + Tags: []*ec2.Tag{ + { + Key: lo.ToPtr("Name"), + Value: lo.ToPtr("test-security-group-2"), + }, + { + Key: lo.ToPtr("foo"), + Value: lo.ToPtr("bar"), + }, + }, + }, + { + GroupId: lo.ToPtr("sg-test3"), + GroupName: lo.ToPtr("securityGroup-test3"), + Tags: []*ec2.Tag{ + { + Key: lo.ToPtr("Name"), + Value: lo.ToPtr("test-security-group-3"), + }, + { + Key: lo.ToPtr("TestTag"), + }, + { + Key: lo.ToPtr("foo"), + Value: lo.ToPtr("bar"), + }, + }, + }, + })) + }() + } + wg.Wait() + }) }) func ExpectConsistsOfSecurityGroups(expected, actual []*ec2.SecurityGroup) { diff --git a/pkg/providers/subnet/subnet.go b/pkg/providers/subnet/subnet.go index 3e115247fee4..c51be4b04946 100644 --- a/pkg/providers/subnet/subnet.go +++ b/pkg/providers/subnet/subnet.go @@ -68,7 +68,9 @@ func (p *Provider) List(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) ([ return nil, err } if subnets, ok := p.cache.Get(fmt.Sprint(hash)); ok { - return subnets.([]*ec2.Subnet), nil + // Ensure what's returned from this function is a shallow-copy of the slice (not a deep-copy of the data itself) + // so that modifications to the ordering of the data don't affect the original + return append([]*ec2.Subnet{}, subnets.([]*ec2.Subnet)...), nil } // Ensure that all the subnets that are returned here are unique diff --git a/pkg/providers/subnet/suite_test.go b/pkg/providers/subnet/suite_test.go index 1c8b8f240e6d..7d9e81970472 100644 --- a/pkg/providers/subnet/suite_test.go +++ b/pkg/providers/subnet/suite_test.go @@ -16,6 +16,8 @@ package subnet_test import ( "context" + "sort" + "sync" "testing" "github.com/aws/aws-sdk-go/aws" @@ -242,6 +244,81 @@ var _ = Describe("SubnetProvider", func() { Expect(onlyPrivate).To(BeTrue()) }) }) + It("should not cause data races when calling List() simultaneously", func() { + wg := sync.WaitGroup{} + for i := 0; i < 10000; i++ { + wg.Add(1) + go func() { + defer wg.Done() + defer GinkgoRecover() + subnets, err := awsEnv.SubnetProvider.List(ctx, nodeClass) + Expect(err).ToNot(HaveOccurred()) + + Expect(subnets).To(HaveLen(3)) + // Sort everything in parallel and ensure that we don't get data races + sort.Slice(subnets, func(i, j int) bool { + if int(*subnets[i].AvailableIpAddressCount) != int(*subnets[j].AvailableIpAddressCount) { + return int(*subnets[i].AvailableIpAddressCount) > int(*subnets[j].AvailableIpAddressCount) + } + return *subnets[i].SubnetId < *subnets[j].SubnetId + }) + Expect(subnets).To(BeEquivalentTo([]*ec2.Subnet{ + { + AvailabilityZone: lo.ToPtr("test-zone-1a"), + AvailableIpAddressCount: lo.ToPtr[int64](100), + SubnetId: lo.ToPtr("subnet-test1"), + MapPublicIpOnLaunch: lo.ToPtr(false), + Tags: []*ec2.Tag{ + { + Key: lo.ToPtr("Name"), + Value: lo.ToPtr("test-subnet-1"), + }, + { + Key: lo.ToPtr("foo"), + Value: lo.ToPtr("bar"), + }, + }, + }, + { + AvailabilityZone: lo.ToPtr("test-zone-1b"), + AvailableIpAddressCount: lo.ToPtr[int64](100), + MapPublicIpOnLaunch: lo.ToPtr(true), + SubnetId: lo.ToPtr("subnet-test2"), + + Tags: []*ec2.Tag{ + { + Key: lo.ToPtr("Name"), + Value: lo.ToPtr("test-subnet-2"), + }, + { + Key: lo.ToPtr("foo"), + Value: lo.ToPtr("bar"), + }, + }, + }, + { + AvailabilityZone: lo.ToPtr("test-zone-1c"), + AvailableIpAddressCount: lo.ToPtr[int64](100), + SubnetId: lo.ToPtr("subnet-test3"), + Tags: []*ec2.Tag{ + { + Key: lo.ToPtr("Name"), + Value: lo.ToPtr("test-subnet-3"), + }, + { + Key: lo.ToPtr("TestTag"), + }, + { + Key: lo.ToPtr("foo"), + Value: lo.ToPtr("bar"), + }, + }, + }, + })) + }() + } + wg.Wait() + }) }) func ExpectConsistsOfSubnets(expected, actual []*ec2.Subnet) {