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 Default Kubernetes Version for Dry-Run #565

Merged
merged 1 commit into from
Aug 21, 2024

Conversation

Alero-Awani
Copy link
Contributor

@Alero-Awani Alero-Awani commented Aug 17, 2024

Description

image (1)

This update improves the handling of Helm charts that do not specify a kubeVersion in their chart.yaml, an example is this redis chart in the image above. The default fallback was Kubernetes version v1.20.0 which affected the rendering of templates and caused this issue.

This change ensures that when kubeVersion is not specified, the Helm dry-run process will fall back to the current version of the Kubernetes cluster in the playground environment.

Notes for Reviewers

Ideally, the version constraint retrieved from chart.Metadata.KubeVersion should not be used to determine the Kubernetes version for the dry run. Instead, the dry-run should utilize the current Kubernetes cluster version to ensure that Helm chart constraints are validated against our cluster's version rather than a potentially outdated version specified in the chart's metadata.

During a Helm dry-run, there is no actual interaction with the live Kubernetes cluster, meaning the process doesn't query the cluster to retrieve the current version. So in this case hardcoding will be better.

Signed commits

  • Yes, I signed my commits.

Copy link

welcome bot commented Aug 17, 2024

Yay, your first pull request! 👍 A contributor will be by to give feedback soon. In the meantime, please review the Layer5 Community Welcome Guide and sure to join the community Slack.
Be sure to double-check that you have signed your commits. Here are instructions for making signing an implicit activity while peforming a commit.

Signed-off-by: Alero Awani <aleroawani18@gmail.com>
@Alero-Awani Alero-Awani force-pushed the alero/meshkit/helm-dry-run branch from 60ff8db to bee904d Compare August 17, 2024 15:33
@MUzairS15
Copy link

@Alero-Awani Similar issue was encountered for one of the other helm chart. To tackle the issue initially I also followed the same approach as you are doing in this PR (falling back to the playground cluster) but it isn’t resilient enough. Hence I shifted to using the helm’s default version. It seems Helm default version also cannot handle the case.

I think in these scenarios, it will be better to use most recent version of the k8s model available in the registry.

@Alero-Awani
Copy link
Contributor Author

Oh yes , I saw the issue. This one here.

You are right, in that case the metadata version is good. Specifically in a scenario where the kubeVersion specified in the chart.yaml is less than(<) the current cluster version.

This particular PR just catches if the kubeVersion was not specified at all in the chart.yaml

I have tested it with a few charts too.

Copy link

@MUzairS15 MUzairS15 left a comment

Choose a reason for hiding this comment

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

@Alero-Awani Let's accept this, I will update to use version from registry in a follow-up PR.

@MUzairS15 MUzairS15 merged commit 836d0eb into meshery:master Aug 21, 2024
2 of 4 checks passed
Copy link

welcome bot commented Aug 21, 2024

Thanks for your contribution to Meshery! 🎉

Shows a black logo in light color mode and a white one in dark color mode.

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