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

Linkerd helm not enforcing PDB values type #13243

Open
bwmetcalf opened this issue Oct 29, 2024 · 2 comments
Open

Linkerd helm not enforcing PDB values type #13243

bwmetcalf opened this issue Oct 29, 2024 · 2 comments
Labels

Comments

@bwmetcalf
Copy link

What is the issue?

The helm chart allows a string or an int for controller.podDisruptionBudget.maxUnavailable and linkerd will happily deploy with, for example, either 1 or 25% for this value and the PDBs are defined accordingly and correctly. However, as soon as a pod starts up where linkerd-proxy needs to be inject, the following error will occur

admission webhook "linkerd-proxy-injector.linkerd.io" denied the request: failed to unmarshal JSON from: /var/run/linkerd/config/values: error unmarshaling JSON: while decoding JSON: json: cannot unmarshal string into Go struct field PodDisruptionBudget.controller.podDisruptionBudget.maxUnavailable of type int

It appears to be related to the strict typing in Go here:

PodDisruptionBudget *PodDisruptionBudget `json:"podDisruptionBudget"`
.

Since k8s allows for percentages for PDBs, it seems the go code should be fixed to allow the same. Either way, if linkerd injection is going to fail if a percent is used in the helm chart values, linkerd should fail to deploy due to the same.

How can it be reproduced?

Define a PDB using a percent, deploy and then attempt to inject linkerd-proxy.

Logs, error output, etc

See initial description.

output of linkerd check -o short

% linkerd check -o short
linkerd-version
---------------
‼ cli is up-to-date
    unsupported version channel: stable-2.14.10
    see https://linkerd.io/2.14/checks/#l5d-version-cli for hints

control-plane-version
---------------------
‼ control plane and cli versions match
    control plane running edge-24.10.4 but cli running stable-2.14.10
    see https://linkerd.io/2.14/checks/#l5d-version-control for hints

linkerd-control-plane-proxy
---------------------------
‼ control plane proxies and cli versions match
    linkerd-destination-6cfbc66dcf-rfqw7 running edge-24.10.4 but cli running stable-2.14.10
    see https://linkerd.io/2.14/checks/#l5d-cp-proxy-cli-version for hints

linkerd-ha-checks
-----------------
‼ pod injection disabled on kube-system
    kube-system namespace needs to have the label config.linkerd.io/admission-webhooks: disabled if injector webhook failure policy is Fail
    see https://linkerd.io/2.14/checks/#l5d-injection-disabled for hints

linkerd-viz
-----------
‼ viz extension proxies and cli versions match
    metrics-api-6ddbbd456-tmtdq running edge-24.10.4 but cli running stable-2.14.10
    see https://linkerd.io/2.14/checks/#l5d-viz-proxy-cli-version for hints

Status check results are √

Environment

% kubectl version
Client Version: v1.28.2
Kustomize Version: v5.0.4-0.20230601165947-6ce0bf390ce3
Server Version: v1.29.8-eks-a737599

Possible solution

No response

Additional context

No response

Would you like to work on fixing this bug?

None

@bwmetcalf bwmetcalf added the bug label Oct 29, 2024
@kflynn
Copy link
Member

kflynn commented Oct 31, 2024

Thanks, @bwmetcalf! As I understand it, actually supporting the "%" form is less important to you than having the chart and the CRD be consistent, correct?

@bwmetcalf
Copy link
Author

It would be ideal to be able to use % so we don't have to change the value for maxUnavailable as we increase or decrease the number of replicas. However, if % isn't going to be supported at injection time, the helm chart should not support it either so it's more consistent with a "fail-fast" paradigm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants