-
Notifications
You must be signed in to change notification settings - Fork 442
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
[Feature] Allow RayCluster Helm chart to specify different images for different worker groups #1352
Conversation
… different worker for different worker groups
… different worker groups
@@ -48,8 +48,13 @@ spec: | |||
containers: | |||
- volumeMounts: {{- toYaml .Values.head.volumeMounts | nindent 12 }} | |||
name: ray-head | |||
{{- if .Values.head.image }} | |||
image: {{ .Values.head.image.repository }}:{{ .Values.head.image.tag }} | |||
imagePullPolicy: {{ .Values.head.image.pullPolicy }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What will happen if pullPolicy
is not defined?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ImagePullPolicy
is an optional field for Pod, and the default value is here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To find out the difference between Pod and this PR regarding the imagePullPolicy
, two test cases are shown below with different values for the tag
of the image
.
In this PR, both the repository
and tag
must be provided when specifying the image
section.
Regarding the imagePullPolicy
:
if it's left unspecified and the tag
isn't set to "latest", the default becomes "IfNotPresent". (As shown in the figure below)
However, if the tag
is "latest" and imagePullPolicy
is not set, the default is "Always". (As shown in the figure below)
When this is merged, will it automatically build a newly available helm chart, or will this be included in a future release (e.g. 0.7.0)? @Darren221, thanks for the work here 😄 and @kevin85421 appreciate your work maintaining the project 😄 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thank @nicklausbrown for reaching out! Currently, CI will not publish the nightly chart automatically, but the first release candidate for Ray Summit (mid Sept) will be out soon. |
… different worker groups (ray-project#1352) Allow RayCluster Helm chart to specify different images for different worker groups
… different worker groups (ray-project#1352) Allow RayCluster Helm chart to specify different images for different worker groups
Why are these changes needed?
Prior to this pull request, the RayCluster Helm chart had uniform default images for various worker groups. This PR introduces a new feature that empowers users to designate distinct images for both worker groups and the head Pod. To ensure backward compatibility, the PR verifies if the "image" section is defined for each Pod. If an image isn't specified in values.yaml, the Pod's image defaults to the option provided at the outset of values.yaml.
Related issue number
Closes #1291
Checks
Test 1:
(Link to values.yaml for test 1)
Test 2:
(Link to values.yaml for test 2)