Robot Upgrade: volcano chart upgrade from 1.9.0 to 1.14.1#3919
Robot Upgrade: volcano chart upgrade from 1.9.0 to 1.14.1#3919weizhoublue wants to merge 1 commit intomainfrom
Conversation
Summary of ChangesHello @weizhoublue, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request performs a significant upgrade of the Volcano Helm chart from version 1.9.0 to 1.14.1. The update introduces several new Custom Resource Definitions (CRDs) to enhance resource colocation, node sharding, and hypernode topology management. It also integrates new Kubernetes Validating and Mutating Admission Policies to enforce robust validation and mutation rules for various Volcano resources. Additionally, the changes include updates to scheduler configurations, Helm chart templates for improved customization and security, and expanded Grafana dashboards for better observability of Volcano's scheduling and resource utilization. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request upgrades the Volcano Helm chart from version 1.9.0 to 1.14.1. The changes are extensive, including updates to CRD definitions, the introduction of new components like agent and agent-scheduler, and a move towards using ValidatingAdmissionPolicy and MutatingAdmissionPolicy for admission control. While this is a significant and valuable upgrade, I've identified a few critical and high-severity issues that need to be addressed. Specifically, there are problems with how container image paths are constructed, leading to invalid paths, and inconsistencies in the version tags used across the chart files. There's also a regression in the configurability of the kube-state-metrics image. Please review the detailed comments for suggestions on how to resolve these issues.
| - -v={{.Values.custom.admission_log_level}} | ||
| - 2>&1 | ||
| image: {{ .Values.admission.image.registry }}/{{ .Values.admission.image.repository }}:{{.Values.admission.image.tag }} | ||
| image: {{ .Values.basic.image_registry }}/{{ .Values.admission.image.registry }}/{{ .Values.admission.image.repository }}:{{.Values.admission.image.tag }} |
There was a problem hiding this comment.
The image path is constructed by concatenating basic.image_registry and admission.image.registry, which results in an invalid image path like docker.io/docker.m.daocloud.io/.... This will cause image pull failures.
A similar issue exists in templates/controllers.yaml and templates/scheduler.yaml.
Please correct the image path construction. You should likely remove the {{ .Values.basic.image_registry }}/ prefix and rely on the component-specific registry value.
image: {{ .Values.admission.image.registry }}/{{ .Values.admission.image.repository }}:{{.Values.admission.image.tag }}| - --logtostderr | ||
| - --enable-healthz=true | ||
| - --leader-elect={{ .Values.custom.leader_elect_enable }} | ||
| image: {{ .Values.basic.image_registry }}/{{ .Values.controller.image.registry }}/{{ .Values.controller.image.repository }}:{{.Values.controller.image.tag }} |
There was a problem hiding this comment.
The image path is constructed by concatenating basic.image_registry and controller.image.registry, which results in an invalid image path like docker.io/docker.m.daocloud.io/.... This will cause image pull failures.
Please correct the image path construction by removing the {{ .Values.basic.image_registry }}/ prefix.
image: {{ .Values.controller.image.registry }}/{{ .Values.controller.image.repository }}:{{.Values.controller.image.tag }}| containers: | ||
| - name: {{ .Release.Name }}-scheduler | ||
| image: {{ .Values.scheduler.image.registry }}/{{ .Values.scheduler.image.repository }}:{{.Values.scheduler.image.tag }} | ||
| image: {{ .Values.basic.image_registry }}/{{ .Values.scheduler.image.registry }}/{{ .Values.scheduler.image.repository }}:{{.Values.scheduler.image.tag }} |
There was a problem hiding this comment.
The image path is constructed by concatenating basic.image_registry and scheduler.image.registry, which results in an invalid image path like docker.io/docker.m.daocloud.io/.... This will cause image pull failures.
Please correct the image path construction by removing the {{ .Values.basic.image_registry }}/ prefix.
image: {{ .Values.scheduler.image.registry }}/{{ .Values.scheduler.image.repository }}:{{.Values.scheduler.image.tag }}| image_pull_secret: "" | ||
| image_pull_policy: "Always" | ||
| image_tag_version: "v1.9.0" | ||
| image_tag_version: "v1.14.0" |
There was a problem hiding this comment.
There is a version inconsistency. The PR upgrades to 1.14.1, which is reflected in Chart.yaml, but this values.yaml file specifies v1.14.0 as the image_tag_version. This should be updated to v1.14.1 for consistency. This also applies to the specific image tags for controller, scheduler, and admission components later in this file.
image_tag_version: "v1.14.1"
charts/volcano/volcano/values.yaml
Outdated
| image_pull_secret: "" | ||
| image_pull_policy: "Always" | ||
| image_tag_version: "v1.9.0" | ||
| image_tag_version: "v1.14.0" |
There was a problem hiding this comment.
There is a version inconsistency. The PR upgrades to 1.14.1, which is reflected in Chart.yaml, but this values.yaml file specifies v1.14.0 as the image_tag_version. This should be updated to v1.14.1 for consistency. This also applies to the specific image tags for controller, scheduler, and admission components later in this file.
image_tag_version: "v1.14.1"| spec: | ||
| containers: | ||
| - image: {{ .Values.kubeStateMetrics.image.registry }}/{{ .Values.kubeStateMetrics.image.repository }}:{{.Values.kubeStateMetrics.image.tag }} | ||
| - image: docker.io/volcanosh/kube-state-metrics:v2.0.0-beta |
There was a problem hiding this comment.
The kube-state-metrics image is now hardcoded. Previously, it was configurable via values.yaml. This change removes the flexibility to customize the image registry, repository, or tag. Please restore the template to use values from values.yaml. You may also need to update the kubeStateMetrics values in values.yaml to use the desired version.
- image: {{ .Values.kubeStateMetrics.image.registry }}/{{ .Values.kubeStateMetrics.image.repository }}:{{.Values.kubeStateMetrics.image.tag }}
github-actions
bot
force-pushed
the
upgrade/volcano/1.14.1
branch
7 times, most recently
from
1a06c62 to
7ca2301
Compare
Signed-off-by: robot <robot@example.com>
github-actions
bot
force-pushed
the
upgrade/volcano/1.14.1
branch
from
7ca2301 to
86587bc
Compare
2 participants
I am robot, upgrade: project volcano chart upgrade from 1.9.0 to 1.14.1