Skip to content

Use cniPluginVersion with Helm for linkerd2-cni#4693

Merged
Pothulapati merged 3 commits intomainfrom
tarun/helm-cni-fix
Jul 2, 2020
Merged

Use cniPluginVersion with Helm for linkerd2-cni#4693
Pothulapati merged 3 commits intomainfrom
tarun/helm-cni-fix

Conversation

@Pothulapati
Copy link
Contributor

Fixes #4691

As cliVersion field is not present when the installation is done through Helm, default to cniVersionValue when it is not present.

Signed-off-by: Tarun Pothulapati [email protected]

use cliVersionVariable as version with Helm, which is overriden by
cliVersion when used with Helm

Signed-off-by: Tarun Pothulapati <[email protected]>
Signed-off-by: Tarun Pothulapati <[email protected]>
@Pothulapati Pothulapati requested a review from a team as a code owner July 1, 2020 08:07
Signed-off-by: Tarun Pothulapati <[email protected]>
k8s-app: linkerd-cni
linkerd.io/cni-resource: "true"
annotations:
linkerd.io/created-by: linkerd/cli dev-undefined
Copy link
Contributor Author

@Pothulapati Pothulapati Jul 1, 2020

Choose a reason for hiding this comment

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

Hmmm, From what I understood, dev-undefined is our fallback when there is no version tag is specified. and whats where we fallback when cliVersion is empty but should we be okay with having tests using this? especially the helm ones as there we can't fall back to this?

@alpeb

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.

Yeah, dev-undefined should be fine for tests 👍

Copy link
Member

@zaharidichev zaharidichev left a comment

Choose a reason for hiding this comment

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

LGTM

@alpeb alpeb mentioned this pull request Jul 2, 2020
@Pothulapati Pothulapati merged commit c3131cd into main Jul 2, 2020
@Pothulapati Pothulapati deleted the tarun/helm-cni-fix branch July 2, 2020 15:15
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.

linkerd-cni helm deployment fails without a values.yaml file

3 participants