Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support to modify liveness and readiness probe timeouts on control plane containers in helm chart #13002

Merged
merged 2 commits into from
Sep 4, 2024

Conversation

kristjankullerkann-wearemp
Copy link
Contributor

@kristjankullerkann-wearemp kristjankullerkann-wearemp commented Aug 29, 2024

This PR adds support for custom liveness and readiness probe timeouts on control plane containers in helm chart.

It was discussed on #12895 with @alpeb

@kristjankullerkann-wearemp kristjankullerkann-wearemp changed the title Add support to modify liveness and readiness probe timeouts on controlplane containers Add support to modify liveness and readiness probe timeouts on control plane containers Aug 29, 2024
@kristjankullerkann-wearemp kristjankullerkann-wearemp marked this pull request as ready for review August 29, 2024 06:40
@kristjankullerkann-wearemp kristjankullerkann-wearemp changed the title Add support to modify liveness and readiness probe timeouts on control plane containers Add support to modify liveness and readiness probe timeouts on control plane containers in helm chart Aug 29, 2024
Copy link
Member

@alpeb alpeb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @kristjankullerkann-wearemp ! 👍
Just couple of comments below.
Also, please don't forget to run bin/helm-docs to re-generate the README chart files with the new values.yaml entries.

charts/linkerd-control-plane/templates/destination.yaml Outdated Show resolved Hide resolved
charts/linkerd-control-plane/values.yaml Outdated Show resolved Hide resolved
@kristjankullerkann-wearemp
Copy link
Contributor Author

kristjankullerkann-wearemp commented Sep 1, 2024

Thanks @kristjankullerkann-wearemp ! 👍 Just couple of comments below. Also, please don't forget to run bin/helm-docs to re-generate the README chart files with the new values.yaml entries.

I did run helm-docs but there wasn't any updates except helm-docs footer itself so I decided not to commit this.

Copy link
Member

@alpeb alpeb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good 👍
Given that those new settings will always be set, at least to their defaults, the tests golden files need to be updated. You can do that with go test ./... -update.

@kristjankullerkann-wearemp
Copy link
Contributor Author

Looking good 👍 Given that those new settings will always be set, at least to their defaults, the tests golden files need to be updated. You can do that with go test ./... -update.

Files updated.

@alpeb
Copy link
Member

alpeb commented Sep 3, 2024

Can you please address the DCO?

@kristjankullerkann-wearemp
Copy link
Contributor Author

Can you please address the DCO?

Sorry, missed signoff for second commit. Should be there now.

@alpeb
Copy link
Member

alpeb commented Sep 3, 2024

There still are missing changes to tests; please apply the following:

diff --git a/cli/cmd/install_test.go b/cli/cmd/install_test.go
index 7d06da27f..2f1e1bd02 100644
--- a/cli/cmd/install_test.go
+++ b/cli/cmd/install_test.go
@@ -60,6 +60,7 @@ func TestRender(t *testing.T) {
                CNIEnabled:              false,
                IdentityTrustDomain:     defaultValues.IdentityTrustDomain,
                IdentityTrustAnchorsPEM: defaultValues.IdentityTrustAnchorsPEM,
+               DestinationController:   map[string]any{},
                PodAnnotations:          map[string]string{},
                PodLabels:               map[string]string{},
                PriorityClassName:       "PriorityClassName",
diff --git a/pkg/charts/linkerd2/values_test.go b/pkg/charts/linkerd2/values_test.go
index ac239ea37..4ac0e8506 100644
--- a/pkg/charts/linkerd2/values_test.go
+++ b/pkg/charts/linkerd2/values_test.go
@@ -98,6 +98,12 @@ func TestNewValues(t *testing.T) {
                                        "while_idle": true,
                                },
                        },
+                       "livenessProbe":  map[string]interface{}{"timeoutSeconds": 1.0},
+                       "readinessProbe": map[string]interface{}{"timeoutSeconds": 1.0},
+               },
+               SPValidator: map[string]interface{}{
+                       "livenessProbe":  map[string]interface{}{"timeoutSeconds": 1.0},
+                       "readinessProbe": map[string]interface{}{"timeoutSeconds": 1.0},
                },
                PolicyController: &PolicyController{
                        Image: &Image{

and then run go test ./... -update one more time.

…l plane containers

Signed-off-by: Kristjan Kullerkann <[email protected]>
@kristjankullerkann-wearemp
Copy link
Contributor Author

There still are missing changes to tests; please apply the following:

diff --git a/cli/cmd/install_test.go b/cli/cmd/install_test.go
index 7d06da27f..2f1e1bd02 100644
--- a/cli/cmd/install_test.go
+++ b/cli/cmd/install_test.go
@@ -60,6 +60,7 @@ func TestRender(t *testing.T) {
                CNIEnabled:              false,
                IdentityTrustDomain:     defaultValues.IdentityTrustDomain,
                IdentityTrustAnchorsPEM: defaultValues.IdentityTrustAnchorsPEM,
+               DestinationController:   map[string]any{},
                PodAnnotations:          map[string]string{},
                PodLabels:               map[string]string{},
                PriorityClassName:       "PriorityClassName",
diff --git a/pkg/charts/linkerd2/values_test.go b/pkg/charts/linkerd2/values_test.go
index ac239ea37..4ac0e8506 100644
--- a/pkg/charts/linkerd2/values_test.go
+++ b/pkg/charts/linkerd2/values_test.go
@@ -98,6 +98,12 @@ func TestNewValues(t *testing.T) {
                                        "while_idle": true,
                                },
                        },
+                       "livenessProbe":  map[string]interface{}{"timeoutSeconds": 1.0},
+                       "readinessProbe": map[string]interface{}{"timeoutSeconds": 1.0},
+               },
+               SPValidator: map[string]interface{}{
+                       "livenessProbe":  map[string]interface{}{"timeoutSeconds": 1.0},
+                       "readinessProbe": map[string]interface{}{"timeoutSeconds": 1.0},
                },
                PolicyController: &PolicyController{
                        Image: &Image{

and then run go test ./... -update one more time.

Done and thanks.

Copy link
Member

@alpeb alpeb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @kristjankullerkann-wearemp ! This looks good to me 👍

@alpeb alpeb merged commit 6f7d3a4 into linkerd:main Sep 4, 2024
39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants