Skip to content

Commit

Permalink
Update nilable duration
Browse files Browse the repository at this point in the history
  • Loading branch information
engedaam committed Aug 12, 2024
1 parent 03be8cc commit 49d5e8a
Show file tree
Hide file tree
Showing 9 changed files with 35 additions and 37 deletions.
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ require (
k8s.io/utils v0.0.0-20240102154912-e7106e64919e
knative.dev/pkg v0.0.0-20231010144348-ca8c009405dd
sigs.k8s.io/controller-runtime v0.18.4
sigs.k8s.io/karpenter v0.36.3-0.20240812071216-7aa2a5c37de7
sigs.k8s.io/karpenter v0.36.3-0.20240812192326-63bfb721100e
sigs.k8s.io/yaml v1.4.0
)

Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -761,10 +761,10 @@ sigs.k8s.io/controller-runtime v0.18.4 h1:87+guW1zhvuPLh1PHybKdYFLU0YJp4FhJRmiHv
sigs.k8s.io/controller-runtime v0.18.4/go.mod h1:TVoGrfdpbA9VRFaRnKgk9P5/atA0pMwq+f+msb9M8Sg=
sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd h1:EDPBXCAspyGV4jQlpZSudPeMmr1bNJefnuqLsRAsHZo=
sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd/go.mod h1:B8JuhiUyNFVKdsE8h686QcCxMaH6HrOAZj4vswFpcB0=
sigs.k8s.io/karpenter v0.36.3-0.20240808011113-f3cd14f1ba19 h1:cvG3uvaF82AZV2g14GMjIhk2Z+Lp3TS/A4vIJ+b2IIw=
sigs.k8s.io/karpenter v0.36.3-0.20240808011113-f3cd14f1ba19/go.mod h1:vZfbuD5UQJfNdaNwiUVgCUJ/yYOVjYbKosr8b971CAM=
sigs.k8s.io/karpenter v0.36.3-0.20240812071216-7aa2a5c37de7 h1:XuaHMCSRIH6NxuXEnJ5XHjU5QdXSS7WNhQz3M74pf/c=
sigs.k8s.io/karpenter v0.36.3-0.20240812071216-7aa2a5c37de7/go.mod h1:vZfbuD5UQJfNdaNwiUVgCUJ/yYOVjYbKosr8b971CAM=
sigs.k8s.io/karpenter v0.36.3-0.20240812192326-63bfb721100e h1:rRUqXMzFCmhyTkJ0LINSOyGK7abS3RHfTp5ZgFbFWN8=
sigs.k8s.io/karpenter v0.36.3-0.20240812192326-63bfb721100e/go.mod h1:vZfbuD5UQJfNdaNwiUVgCUJ/yYOVjYbKosr8b971CAM=
sigs.k8s.io/structured-merge-diff/v4 v4.4.1 h1:150L+0vs/8DA78h1u02ooW1/fFq/Lwr+sGiqlzvrtq4=
sigs.k8s.io/structured-merge-diff/v4 v4.4.1/go.mod h1:N8hJocpFajUSSeSJ9bOZ77VzejKZaXsTtZo4/u7Io08=
sigs.k8s.io/yaml v1.4.0 h1:Mk1wCc2gy/F0THH0TAp1QYyJNzRm2KCLy3o5ASXVI5E=
Expand Down
4 changes: 2 additions & 2 deletions test/pkg/environment/common/environment.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,8 @@ func (env *Environment) DefaultNodePool(nodeClass *v1beta1.EC2NodeClass) *corev1
},
},
}
nodePool.Spec.Disruption.ConsolidateAfter = &corev1beta1.NillableDuration{}
nodePool.Spec.Disruption.ExpireAfter.Duration = nil
nodePool.Spec.Disruption.ConsolidateAfter = lo.ToPtr(corev1beta1.MustParseNillableDuration("Never"))
nodePool.Spec.Disruption.ExpireAfter = corev1beta1.MustParseNillableDuration("Never")
nodePool.Spec.Limits = corev1beta1.Limits(v1.ResourceList{
v1.ResourceCPU: resource.MustParse("1000"),
v1.ResourceMemory: resource.MustParse("1000Gi"),
Expand Down
4 changes: 2 additions & 2 deletions test/suites/chaos/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ var _ = Describe("Chaos", func() {
},
})
nodePool.Spec.Disruption.ConsolidationPolicy = corev1beta1.ConsolidationPolicyWhenUnderutilized
nodePool.Spec.Disruption.ConsolidateAfter = nil
nodePool.Spec.Disruption.ConsolidateAfter = lo.ToPtr(corev1beta1.MustParseNillableDuration("Never"))

numPods := 1
dep := coretest.Deployment(coretest.DeploymentOptions{
Expand Down Expand Up @@ -113,7 +113,7 @@ var _ = Describe("Chaos", func() {
defer cancel()

nodePool.Spec.Disruption.ConsolidationPolicy = corev1beta1.ConsolidationPolicyWhenEmpty
nodePool.Spec.Disruption.ConsolidateAfter = &corev1beta1.NillableDuration{Duration: lo.ToPtr(30 * time.Second)}
nodePool.Spec.Disruption.ConsolidateAfter = lo.ToPtr(corev1beta1.MustParseNillableDuration("30s"))
numPods := 1
dep := coretest.Deployment(coretest.DeploymentOptions{
Replicas: int32(numPods),
Expand Down
10 changes: 5 additions & 5 deletions test/suites/consolidation/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ var _ = Describe("Consolidation", func() {
})
It("should respect budgets for non-empty delete consolidation", func() {
// This test will hold consolidation until we are ready to execute it
nodePool.Spec.Disruption.ConsolidateAfter = &corev1beta1.NillableDuration{}
nodePool.Spec.Disruption.ConsolidateAfter = lo.ToPtr(corev1beta1.MustParseNillableDuration("Never"))

nodePool = test.ReplaceRequirements(nodePool,
corev1beta1.NodeSelectorRequirementWithMinValues{
Expand Down Expand Up @@ -220,7 +220,7 @@ var _ = Describe("Consolidation", func() {
It("should respect budgets for non-empty replace consolidation", func() {
appLabels := map[string]string{"app": "large-app"}
// This test will hold consolidation until we are ready to execute it
nodePool.Spec.Disruption.ConsolidateAfter = &corev1beta1.NillableDuration{}
nodePool.Spec.Disruption.ConsolidateAfter = lo.ToPtr(corev1beta1.MustParseNillableDuration("Never"))

nodePool = test.ReplaceRequirements(nodePool,
corev1beta1.NodeSelectorRequirementWithMinValues{
Expand Down Expand Up @@ -403,7 +403,7 @@ var _ = Describe("Consolidation", func() {
Disruption: corev1beta1.Disruption{
ConsolidationPolicy: corev1beta1.ConsolidationPolicyWhenUnderutilized,
// Disable Consolidation until we're ready
ConsolidateAfter: &corev1beta1.NillableDuration{},
ConsolidateAfter: lo.ToPtr(corev1beta1.MustParseNillableDuration("Never")),
},
Template: corev1beta1.NodeClaimTemplate{
Spec: corev1beta1.NodeClaimSpec{
Expand Down Expand Up @@ -482,7 +482,7 @@ var _ = Describe("Consolidation", func() {
Disruption: corev1beta1.Disruption{
ConsolidationPolicy: corev1beta1.ConsolidationPolicyWhenUnderutilized,
// Disable Consolidation until we're ready
ConsolidateAfter: &corev1beta1.NillableDuration{},
ConsolidateAfter: lo.ToPtr(corev1beta1.MustParseNillableDuration("Never")),
},
Template: corev1beta1.NodeClaimTemplate{
Spec: corev1beta1.NodeClaimSpec{
Expand Down Expand Up @@ -618,7 +618,7 @@ var _ = Describe("Consolidation", func() {
Disruption: corev1beta1.Disruption{
ConsolidationPolicy: corev1beta1.ConsolidationPolicyWhenUnderutilized,
// Disable Consolidation until we're ready
ConsolidateAfter: &corev1beta1.NillableDuration{},
ConsolidateAfter: lo.ToPtr(corev1beta1.MustParseNillableDuration("Never")),
},
Template: corev1beta1.NodeClaimTemplate{
Spec: corev1beta1.NodeClaimSpec{
Expand Down
24 changes: 12 additions & 12 deletions test/suites/expiration/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ var _ = BeforeEach(func() {
env.BeforeEach()
nodeClass = env.DefaultEC2NodeClass()
nodePool = env.DefaultNodePool(nodeClass)
nodePool.Spec.Disruption.ExpireAfter = corev1beta1.NillableDuration{Duration: lo.ToPtr(time.Second * 30)}
nodePool.Spec.Disruption.ExpireAfter = corev1beta1.MustParseNillableDuration("30s")
})

var _ = AfterEach(func() { env.Cleanup() })
Expand Down Expand Up @@ -108,7 +108,7 @@ var _ = Describe("Expiration", func() {
nodePool.Spec.Disruption.Budgets = []corev1beta1.Budget{{
Nodes: "100%",
}}
nodePool.Spec.Disruption.ExpireAfter = corev1beta1.NillableDuration{}
nodePool.Spec.Disruption.ExpireAfter = corev1beta1.MustParseNillableDuration("Never")

// Create a deployment with one pod to create one node.
dep = coretest.Deployment(coretest.DeploymentOptions{
Expand Down Expand Up @@ -157,7 +157,7 @@ var _ = Describe("Expiration", func() {
env.ExpectUpdated(node)

By("enabling expiration")
nodePool.Spec.Disruption.ExpireAfter = corev1beta1.NillableDuration{Duration: lo.ToPtr(time.Second * 30)}
nodePool.Spec.Disruption.ExpireAfter = corev1beta1.MustParseNillableDuration("30s")
env.ExpectUpdated(nodePool)

// Expect that both of the nodes are expired, but not being disrupted
Expand Down Expand Up @@ -191,7 +191,7 @@ var _ = Describe("Expiration", func() {
nodePool.Spec.Disruption.Budgets = []corev1beta1.Budget{{
Nodes: "50%",
}}
nodePool.Spec.Disruption.ExpireAfter = corev1beta1.NillableDuration{}
nodePool.Spec.Disruption.ExpireAfter = corev1beta1.MustParseNillableDuration("Never")

numPods = 6
dep = coretest.Deployment(coretest.DeploymentOptions{
Expand Down Expand Up @@ -232,7 +232,7 @@ var _ = Describe("Expiration", func() {
env.ExpectDeleted(dep)

By("enabling expiration")
nodePool.Spec.Disruption.ExpireAfter = corev1beta1.NillableDuration{Duration: lo.ToPtr(time.Second * 30)}
nodePool.Spec.Disruption.ExpireAfter = corev1beta1.MustParseNillableDuration("30s")
env.ExpectUpdated(nodePool)

env.EventuallyExpectExpired(nodeClaims...)
Expand Down Expand Up @@ -272,7 +272,7 @@ var _ = Describe("Expiration", func() {
Nodes: "50%",
}}
// disable expiration so that we can enable it later when we want.
nodePool.Spec.Disruption.ExpireAfter = corev1beta1.NillableDuration{}
nodePool.Spec.Disruption.ExpireAfter = corev1beta1.MustParseNillableDuration("Never")
numPods = 9
dep = coretest.Deployment(coretest.DeploymentOptions{
Replicas: int32(numPods),
Expand Down Expand Up @@ -318,7 +318,7 @@ var _ = Describe("Expiration", func() {

By("expiring the nodes")
// expire the nodeclaims
nodePool.Spec.Disruption.ExpireAfter = corev1beta1.NillableDuration{Duration: lo.ToPtr(time.Second * 30)}
nodePool.Spec.Disruption.ExpireAfter = corev1beta1.MustParseNillableDuration("30s")
env.ExpectUpdated(nodePool)

env.EventuallyExpectExpired(nodeClaims...)
Expand Down Expand Up @@ -408,7 +408,7 @@ var _ = Describe("Expiration", func() {
}

By("enabling expiration")
nodePool.Spec.Disruption.ExpireAfter = corev1beta1.NillableDuration{Duration: lo.ToPtr(30 * time.Second)}
nodePool.Spec.Disruption.ExpireAfter = corev1beta1.MustParseNillableDuration("30s")
env.ExpectUpdated(nodePool)

// Ensure that we get two nodes tainted, and they have overlap during the expiration
Expand All @@ -420,7 +420,7 @@ var _ = Describe("Expiration", func() {
// Set the expireAfter to "Never" to make sure new node isn't deleted
// This is CRITICAL since it prevents nodes that are immediately spun up from immediately being expired and
// racing at the end of the E2E test, leaking node resources into subsequent tests
nodePool.Spec.Disruption.ExpireAfter.Duration = nil
nodePool.Spec.Disruption.ExpireAfter = corev1beta1.MustParseNillableDuration("Never")
env.ExpectUpdated(nodePool)

for _, node := range nodes {
Expand Down Expand Up @@ -497,7 +497,7 @@ var _ = Describe("Expiration", func() {
// Set the expireAfter to "Never" to make sure new node isn't deleted
// This is CRITICAL since it prevents nodes that are immediately spun up from immediately being expired and
// racing at the end of the E2E test, leaking node resources into subsequent tests
nodePool.Spec.Disruption.ExpireAfter.Duration = nil
nodePool.Spec.Disruption.ExpireAfter = corev1beta1.MustParseNillableDuration("Never")
env.ExpectUpdated(nodePool)

// After the deletion timestamp is set and all pods are drained
Expand Down Expand Up @@ -538,7 +538,7 @@ var _ = Describe("Expiration", func() {
env.Monitor.Reset() // Reset the monitor so that we can expect a single node to be spun up after expiration

// Set the expireAfter value to get the node deleted
nodePool.Spec.Disruption.ExpireAfter.Duration = lo.ToPtr(time.Minute)
nodePool.Spec.Disruption.ExpireAfter = corev1beta1.MustParseNillableDuration("1m")
env.ExpectUpdated(nodePool)

env.EventuallyExpectExpired(nodeClaim)
Expand All @@ -561,7 +561,7 @@ var _ = Describe("Expiration", func() {
// Set the expireAfter to "Never" to make sure new node isn't deleted
// This is CRITICAL since it prevents nodes that are immediately spun up from immediately being expired and
// racing at the end of the E2E test, leaking node resources into subsequent tests
nodePool.Spec.Disruption.ExpireAfter.Duration = nil
nodePool.Spec.Disruption.ExpireAfter = corev1beta1.MustParseNillableDuration("1m")
env.ExpectUpdated(nodePool)

// After the deletion timestamp is set and all pods are drained
Expand Down
6 changes: 3 additions & 3 deletions test/suites/integration/emptiness_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ var _ = Describe("Emptiness", func() {
var numPods int
BeforeEach(func() {
nodePool.Spec.Disruption.ConsolidationPolicy = corev1beta1.ConsolidationPolicyWhenEmpty
nodePool.Spec.Disruption.ConsolidateAfter = &corev1beta1.NillableDuration{Duration: lo.ToPtr(time.Duration(0))}
nodePool.Spec.Disruption.ConsolidateAfter = lo.ToPtr(corev1beta1.MustParseNillableDuration("0s"))

numPods = 1
dep = test.Deployment(test.DeploymentOptions{
Expand Down Expand Up @@ -97,7 +97,7 @@ var _ = Describe("Emptiness", func() {
})
})
It("should terminate an empty node", func() {
nodePool.Spec.Disruption.ConsolidateAfter = &corev1beta1.NillableDuration{Duration: lo.ToPtr(time.Hour * 300)}
nodePool.Spec.Disruption.ConsolidateAfter = lo.ToPtr(corev1beta1.MustParseNillableDuration("300h"))

const numPods = 1
deployment := test.Deployment(test.DeploymentOptions{Replicas: numPods})
Expand All @@ -116,7 +116,7 @@ var _ = Describe("Emptiness", func() {
env.EventuallyExpectEmpty(nodeClaim)

By("waiting for the nodeclaim to deprovision when past its ConsolidateAfter timeout of 0")
nodePool.Spec.Disruption.ConsolidateAfter = &corev1beta1.NillableDuration{Duration: lo.ToPtr(time.Duration(0))}
nodePool.Spec.Disruption.ConsolidateAfter = lo.ToPtr(corev1beta1.MustParseNillableDuration("0s"))
env.ExpectUpdated(nodePool)

env.EventuallyExpectNotFound(nodeClaim, node)
Expand Down
5 changes: 2 additions & 3 deletions test/suites/integration/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ package integration_test

import (
"fmt"
"time"

"github.com/samber/lo"
v1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -104,12 +103,12 @@ var _ = Describe("Validation", func() {
})
It("should error when ttlSecondAfterEmpty is negative", func() {
nodePool.Spec.Disruption.ConsolidationPolicy = corev1beta1.ConsolidationPolicyWhenEmpty
nodePool.Spec.Disruption.ConsolidateAfter = &corev1beta1.NillableDuration{Duration: lo.ToPtr(-time.Second)}
nodePool.Spec.Disruption.ConsolidateAfter = lo.ToPtr(corev1beta1.MustParseNillableDuration("-1s"))
Expect(env.Client.Create(env.Context, nodePool)).ToNot(Succeed())
})
It("should error when ConsolidationPolicy=WhenUnderutilized is used with consolidateAfter", func() {
nodePool.Spec.Disruption.ConsolidationPolicy = corev1beta1.ConsolidationPolicyWhenUnderutilized
nodePool.Spec.Disruption.ConsolidateAfter = &corev1beta1.NillableDuration{Duration: lo.ToPtr(time.Minute)}
nodePool.Spec.Disruption.ConsolidateAfter = lo.ToPtr(corev1beta1.MustParseNillableDuration("1m"))
Expect(env.Client.Create(env.Context, nodePool)).ToNot(Succeed())
})
It("should error if imageGCHighThresholdPercent is less than imageGCLowThresholdPercent", func() {
Expand Down
13 changes: 6 additions & 7 deletions test/suites/scale/deprovisioning_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/util/uuid"
"knative.dev/pkg/ptr"

corev1beta1 "sigs.k8s.io/karpenter/pkg/apis/v1beta1"
"sigs.k8s.io/karpenter/pkg/test"
Expand Down Expand Up @@ -236,8 +235,8 @@ var _ = Describe("Deprovisioning", Label(debug.NoWatch), Label(debug.NoEvents),
// Enable consolidation, emptiness, and expiration
nodePoolMap[consolidationValue].Spec.Disruption.ConsolidateAfter = nil
nodePoolMap[emptinessValue].Spec.Disruption.ConsolidationPolicy = corev1beta1.ConsolidationPolicyWhenEmpty
nodePoolMap[emptinessValue].Spec.Disruption.ConsolidateAfter.Duration = ptr.Duration(0)
nodePoolMap[expirationValue].Spec.Disruption.ExpireAfter.Duration = ptr.Duration(0)
nodePoolMap[emptinessValue].Spec.Disruption.ConsolidateAfter = lo.ToPtr(corev1beta1.MustParseNillableDuration("0s"))
nodePoolMap[expirationValue].Spec.Disruption.ExpireAfter = corev1beta1.MustParseNillableDuration("0s")
nodePoolMap[expirationValue].Spec.Limits = disableProvisioningLimits
// Update the drift NodeClass to start drift on Nodes assigned to this NodeClass
driftNodeClass.Spec.AMIFamily = &v1beta1.AMIFamilyBottlerocket
Expand Down Expand Up @@ -545,7 +544,7 @@ var _ = Describe("Deprovisioning", Label(debug.NoWatch), Label(debug.NoEvents),
env.MeasureDeprovisioningDurationFor(func() {
By("kicking off deprovisioning emptiness by setting the ttlSecondsAfterEmpty value on the nodePool")
nodePool.Spec.Disruption.ConsolidationPolicy = corev1beta1.ConsolidationPolicyWhenEmpty
nodePool.Spec.Disruption.ConsolidateAfter.Duration = ptr.Duration(0)
nodePool.Spec.Disruption.ConsolidateAfter = lo.ToPtr(corev1beta1.MustParseNillableDuration("0s"))
env.ExpectCreatedOrUpdated(nodePool)

env.EventuallyExpectDeletedNodeCount("==", expectedNodeCount)
Expand Down Expand Up @@ -598,13 +597,13 @@ var _ = Describe("Deprovisioning", Label(debug.NoWatch), Label(debug.NoEvents),
// Change limits so that replacement nodes will use another nodePool.
nodePool.Spec.Limits = disableProvisioningLimits
// Enable Expiration
nodePool.Spec.Disruption.ExpireAfter.Duration = ptr.Duration(0)
nodePool.Spec.Disruption.ExpireAfter = corev1beta1.MustParseNillableDuration("0s")

noExpireNodePool := test.NodePool(*nodePool.DeepCopy())

// Disable Expiration
noExpireNodePool.Spec.Disruption.ConsolidateAfter = &corev1beta1.NillableDuration{}
noExpireNodePool.Spec.Disruption.ExpireAfter.Duration = nil
noExpireNodePool.Spec.Disruption.ConsolidateAfter = lo.ToPtr(corev1beta1.MustParseNillableDuration("Never"))
noExpireNodePool.Spec.Disruption.ExpireAfter = corev1beta1.MustParseNillableDuration("Never")

noExpireNodePool.ObjectMeta = metav1.ObjectMeta{Name: test.RandomName()}
noExpireNodePool.Spec.Template.Spec.Kubelet = &corev1beta1.KubeletConfiguration{
Expand Down

0 comments on commit 49d5e8a

Please sign in to comment.