From 22580290de03f5cecd7994ef134cc30c7081c07f Mon Sep 17 00:00:00 2001 From: Jonathan Innis Date: Thu, 25 Jan 2024 14:17:55 -0800 Subject: [PATCH] PR comments --- .../karpenter.k8s.aws_ec2nodeclasses.yaml | 322 ++++++++++-------- test/pkg/environment/common/expectations.go | 21 +- test/suites/consolidation/suite_test.go | 16 +- test/suites/drift/suite_test.go | 14 +- test/suites/expiration/suite_test.go | 14 +- test/suites/integration/emptiness_test.go | 4 +- 6 files changed, 225 insertions(+), 166 deletions(-) diff --git a/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml b/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml index 2beec0384c5d..af4acba6004c 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.14.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 @@ -215,76 +247,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 @@ -295,24 +342,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: @@ -336,9 +384,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 @@ -347,9 +395,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: @@ -384,10 +432,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 @@ -410,8 +458,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 @@ -426,26 +475,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 @@ -464,8 +512,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 @@ -481,8 +530,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/test/pkg/environment/common/expectations.go b/test/pkg/environment/common/expectations.go index 81873347a591..466438ee49d0 100644 --- a/test/pkg/environment/common/expectations.go +++ b/test/pkg/environment/common/expectations.go @@ -491,7 +491,7 @@ func NodeNames(nodes []*v1.Node) []string { }) } -func (env *Environment) ConsistentlyExpectNodeCount(comparator string, count int, duration string) []*v1.Node { +func (env *Environment) ConsistentlyExpectNodeCount(comparator string, count int, duration time.Duration) []*v1.Node { GinkgoHelper() By(fmt.Sprintf("expecting nodes to be %s to %d for %s", comparator, count, duration)) nodeList := &v1.NodeList{} @@ -499,14 +499,14 @@ func (env *Environment) ConsistentlyExpectNodeCount(comparator string, count int g.Expect(env.Client.List(env, nodeList, client.HasLabels{test.DiscoveryLabel})).To(Succeed()) g.Expect(len(nodeList.Items)).To(BeNumerically(comparator, count), fmt.Sprintf("expected %d nodes, had %d (%v) for %s", count, len(nodeList.Items), NodeNames(lo.ToSlicePtr(nodeList.Items)), duration)) - }, duration).Should(Succeed()) + }, duration.String()).Should(Succeed()) return lo.ToSlicePtr(nodeList.Items) } // ConsistentlyExpectNoDisruptions ensures that the state of the cluster is not changed within a passed duration // Specifically, we check if the cluster size in terms of nodes is the same as the passed-in size and we validate // that no disrupting taints are added throughout the window -func (env *Environment) ConsistentlyExpectNoDisruptions(nodeCount int, duration string) { +func (env *Environment) ConsistentlyExpectNoDisruptions(nodeCount int, duration time.Duration) { GinkgoHelper() Consistently(func(g Gomega) { // Ensure we don't change our NodeClaims @@ -524,10 +524,10 @@ func (env *Environment) ConsistentlyExpectNoDisruptions(nodeCount int, duration }) g.Expect(ok).To(BeFalse()) } - }, duration).Should(Succeed()) + }, duration.String()).Should(Succeed()) } -func (env *Environment) ConsistentlyExpectTaintedNodeCount(comparator string, count int, duration string) []*v1.Node { +func (env *Environment) ConsistentlyExpectTaintedNodeCount(comparator string, count int, duration time.Duration) []*v1.Node { GinkgoHelper() By(fmt.Sprintf("checking for tainted nodes to be %s to %d for %s", comparator, count, duration)) @@ -536,7 +536,7 @@ func (env *Environment) ConsistentlyExpectTaintedNodeCount(comparator string, co g.Expect(env.Client.List(env, nodeList, client.MatchingFields{"spec.taints[*].karpenter.sh/disruption": "disrupting"})).To(Succeed()) g.Expect(len(nodeList.Items)).To(BeNumerically(comparator, count), fmt.Sprintf("expected %d tainted nodes, had %d (%v)", count, len(nodeList.Items), NodeNames(lo.ToSlicePtr(nodeList.Items)))) - }, duration).Should(Succeed()) + }, duration.String()).Should(Succeed()) return lo.ToSlicePtr(nodeList.Items) } @@ -798,6 +798,7 @@ func (env *Environment) ForcePodsToSpread(nodes ...*v1.Node) { maxPodsPerNode := int(math.Ceil(float64(podCount) / float64(len(nodes)))) By(fmt.Sprintf("forcing %d pods to spread across %d nodes", podCount, len(nodes))) + start := time.Now() for { var nodePods []*v1.Pod node, found := lo.Find(nodes, func(n *v1.Node) bool { @@ -818,6 +819,13 @@ func (env *Environment) ForcePodsToSpread(nodes ...*v1.Node) { Eventually(func(g Gomega) { g.Expect(len(env.ExpectActivePodsForNode(node.Name))).To(Or(Equal(maxPodsPerNode), Equal(maxPodsPerNode-1))) }).WithTimeout(5 * time.Second).Should(Succeed()) + + // TODO: Consider moving this time check to an Eventually poll. This gets a little tricker with helper functions + // since you need to make sure that your Expectation helper functions are scoped to to your "g Gomega" scope + // so that you don't fail the first time you get a failure on your expectation + if time.Since(start) > time.Minute*15 { + Fail("forcing pods to spread failed due to a timeout") + } } for _, n := range nodes { stored := n.DeepCopy() @@ -831,7 +839,6 @@ func (env *Environment) ExpectActivePodsForNode(nodeName string) []*v1.Pod { podList := &v1.PodList{} Expect(env.Client.List(env, podList, client.MatchingFields{"spec.nodeName": nodeName}, client.HasLabels{test.DiscoveryLabel})).To(Succeed()) - // Return the healthy pods return lo.Filter(lo.ToSlicePtr(podList.Items), func(p *v1.Pod, _ int) bool { return p.DeletionTimestamp.IsZero() }) diff --git a/test/suites/consolidation/suite_test.go b/test/suites/consolidation/suite_test.go index 7754ab55328b..5a5bfe1502b0 100644 --- a/test/suites/consolidation/suite_test.go +++ b/test/suites/consolidation/suite_test.go @@ -125,8 +125,8 @@ var _ = Describe("Consolidation", func() { env.ExpectUpdated(dep) // Ensure that we get two nodes tainted, and they have overlap during the drift - nodes = env.EventuallyExpectTaintedNodeCount("==", 2) - env.ConsistentlyExpectTaintedNodeCount("==", 2, "5s") + env.EventuallyExpectTaintedNodeCount("==", 2) + nodes = env.ConsistentlyExpectTaintedNodeCount("==", 2, time.Second*5) // Remove the finalizer from each node so that we can terminate for _, node := range nodes { @@ -138,7 +138,9 @@ var _ = Describe("Consolidation", func() { env.EventuallyExpectNotFound(nodes[0], nodes[1]) // This check ensures that we are consolidating nodes at the same time - nodes = env.EventuallyExpectTaintedNodeCount("==", 2) + env.EventuallyExpectTaintedNodeCount("==", 2) + nodes = env.ConsistentlyExpectTaintedNodeCount("==", 2, time.Second*5) + for _, node := range nodes { Expect(env.ExpectTestingFinalizerRemoved(node)).To(Succeed()) } @@ -205,8 +207,8 @@ var _ = Describe("Consolidation", func() { env.ExpectUpdated(nodePool) // Ensure that we get two nodes tainted, and they have overlap during the drift - nodes = env.EventuallyExpectTaintedNodeCount("==", 2) - env.ConsistentlyExpectTaintedNodeCount("==", 2, "5s") + env.EventuallyExpectTaintedNodeCount("==", 2) + nodes = env.ConsistentlyExpectTaintedNodeCount("==", 2, time.Second*5) for _, node := range nodes { Expect(env.ExpectTestingFinalizerRemoved(node)).To(Succeed()) @@ -242,7 +244,7 @@ var _ = Describe("Consolidation", func() { // Update the deployment to only contain 1 replica. env.ExpectUpdated(dep) - env.ConsistentlyExpectNoDisruptions(5, "1m") + env.ConsistentlyExpectNoDisruptions(5, time.Minute) }) It("should not allow consolidation if the budget is fully blocking during a scheduled time", func() { // We're going to define a budget that doesn't allow any drift to happen @@ -278,7 +280,7 @@ var _ = Describe("Consolidation", func() { // Update the deployment to only contain 1 replica. env.ExpectUpdated(dep) - env.ConsistentlyExpectNoDisruptions(5, "1m") + env.ConsistentlyExpectNoDisruptions(5, time.Minute) }) }) DescribeTable("should consolidate nodes (delete)", Label(debug.NoWatch), Label(debug.NoEvents), diff --git a/test/suites/drift/suite_test.go b/test/suites/drift/suite_test.go index bbc512e0f269..fc74576496d5 100644 --- a/test/suites/drift/suite_test.go +++ b/test/suites/drift/suite_test.go @@ -158,8 +158,8 @@ var _ = Describe("Drift", func() { env.EventuallyExpectDrifted(nodeClaims...) // Ensure that we get two nodes tainted, and they have overlap during the drift - nodes = env.EventuallyExpectTaintedNodeCount("==", 2) - env.ConsistentlyExpectTaintedNodeCount("==", 2, "5s") + env.EventuallyExpectTaintedNodeCount("==", 2) + nodes = env.ConsistentlyExpectTaintedNodeCount("==", 2, time.Second*5) // Remove the finalizer from each node so that we can terminate for _, node := range nodes { @@ -250,8 +250,8 @@ var _ = Describe("Drift", func() { env.ExpectUpdated(nodes[0]) // Ensure that we get two nodes tainted, and they have overlap during the drift - nodes = env.EventuallyExpectTaintedNodeCount("==", 2) - env.ConsistentlyExpectTaintedNodeCount("==", 2, "5s") + env.EventuallyExpectTaintedNodeCount("==", 2) + nodes = env.ConsistentlyExpectTaintedNodeCount("==", 2, time.Second*5) By("removing the finalizer from the nodes") Expect(env.ExpectTestingFinalizerRemoved(nodes[0])).To(Succeed()) @@ -280,7 +280,7 @@ var _ = Describe("Drift", func() { env.ExpectUpdated(nodePool) env.EventuallyExpectDrifted(nodeClaim) - env.ConsistentlyExpectNoDisruptions(1, "1m") + env.ConsistentlyExpectNoDisruptions(1, time.Minute) }) It("should not allow drift if the budget is fully blocking during a scheduled time", func() { // We're going to define a budget that doesn't allow any drift to happen @@ -307,7 +307,7 @@ var _ = Describe("Drift", func() { env.ExpectUpdated(nodePool) env.EventuallyExpectDrifted(nodeClaim) - env.ConsistentlyExpectNoDisruptions(1, "1m") + env.ConsistentlyExpectNoDisruptions(1, time.Minute) }) }) It("should disrupt nodes that have drifted due to AMIs", func() { @@ -782,7 +782,7 @@ var _ = Describe("Drift", func() { env.ExpectUpdated(nodePool) env.EventuallyExpectDrifted(nodeClaims...) - env.ConsistentlyExpectNoDisruptions(int(numPods), "1m") + env.ConsistentlyExpectNoDisruptions(int(numPods), time.Minute) }) }) }) diff --git a/test/suites/expiration/suite_test.go b/test/suites/expiration/suite_test.go index a2d33ef731fb..d66fd195c7f8 100644 --- a/test/suites/expiration/suite_test.go +++ b/test/suites/expiration/suite_test.go @@ -151,8 +151,8 @@ var _ = Describe("Expiration", func() { env.EventuallyExpectExpired(nodeClaims...) // Expect that two nodes are tainted. - nodes = env.EventuallyExpectTaintedNodeCount("==", 2) - env.ConsistentlyExpectTaintedNodeCount("==", 2, "5s") + env.EventuallyExpectTaintedNodeCount("==", 2) + nodes = env.ConsistentlyExpectTaintedNodeCount("==", 2, time.Second*5) // Remove finalizers for _, node := range nodes { @@ -248,8 +248,8 @@ var _ = Describe("Expiration", func() { env.ExpectUpdated(nodes[0]) // Ensure that we get two nodes tainted, and they have overlap during the expiration - nodes = env.EventuallyExpectTaintedNodeCount("==", 2) - env.ConsistentlyExpectTaintedNodeCount("==", 2, "5s") + env.EventuallyExpectTaintedNodeCount("==", 2) + nodes = env.ConsistentlyExpectTaintedNodeCount("==", 2, time.Second*5) By("removing the finalizer from the nodes") Expect(env.ExpectTestingFinalizerRemoved(nodes[0])).To(Succeed()) @@ -273,7 +273,7 @@ var _ = Describe("Expiration", func() { env.EventuallyExpectHealthyPodCount(selector, numPods) env.EventuallyExpectExpired(nodeClaim) - env.ConsistentlyExpectNoDisruptions(1, "1m") + env.ConsistentlyExpectNoDisruptions(1, time.Minute) }) It("should not allow expiration if the budget is fully blocking during a scheduled time", func() { // We're going to define a budget that doesn't allow any expirations to happen @@ -295,7 +295,7 @@ var _ = Describe("Expiration", func() { env.EventuallyExpectHealthyPodCount(selector, numPods) env.EventuallyExpectExpired(nodeClaim) - env.ConsistentlyExpectNoDisruptions(1, "1m") + env.ConsistentlyExpectNoDisruptions(1, time.Minute) }) }) It("should expire the node after the expiration is reached", func() { @@ -560,7 +560,7 @@ var _ = Describe("Expiration", func() { env.EventuallyExpectBoundPodCount(selector, int(numPods)) env.EventuallyExpectExpired(nodeClaims...) - env.ConsistentlyExpectNoDisruptions(int(numPods), "1m") + env.ConsistentlyExpectNoDisruptions(int(numPods), time.Minute) }) }) }) diff --git a/test/suites/integration/emptiness_test.go b/test/suites/integration/emptiness_test.go index d583e4a07b11..ec74592e6513 100644 --- a/test/suites/integration/emptiness_test.go +++ b/test/suites/integration/emptiness_test.go @@ -69,7 +69,7 @@ var _ = Describe("Emptiness", func() { env.ExpectDeleted(dep) env.EventuallyExpectEmpty(nodeClaim) - env.ConsistentlyExpectNoDisruptions(1, "1m") + env.ConsistentlyExpectNoDisruptions(1, time.Minute) }) It("should not allow emptiness if the budget is fully blocking during a scheduled time", func() { // We're going to define a budget that doesn't allow any emptiness disruption to happen @@ -93,7 +93,7 @@ var _ = Describe("Emptiness", func() { env.ExpectDeleted(dep) env.EventuallyExpectEmpty(nodeClaim) - env.ConsistentlyExpectNoDisruptions(1, "1m") + env.ConsistentlyExpectNoDisruptions(1, time.Minute) }) }) It("should terminate an empty node", func() {