Add tag config for server/ui/admintools images #789
Conversation
|
|
I don't know why deepcopy files are changed. I tried go 1.22.6 / 1.23.0 on mac (arm64). Should I just remove that change? |
| @@ -1002,6 +1009,9 @@ type TemporalClusterSpec struct { | |||
| // Image defines the temporal server docker image the cluster should use for each services. | |||
| // +optional | |||
| Image string `json:"image"` | |||
| // Tag defines the temporal server docker image tag the cluster should use for each services. | |||
| // +optional | |||
| Tag string `json:"tag"` | |||
There was a problem hiding this comment.
IMO adding .spec.tag in the API can be confusing for end users.
The .spec.version is used as a source of truth on the whole codebase. Please remove this.
There was a problem hiding this comment.
Users of this operator may not be able to change the convention of container images. Therefore making assumption that tag will be same with version is not desired.
For example I have to use x.y version with custom repo & tag (x.y-yyyymmdd) then there is no way to do that currently. Adding tag solves the problem.
Also I made it optional, falling back to version value - which keeps the current behavior.
Another approach I considered is to introduce image struct with repo and tag fields - instead of having them flattened in the spec. We may move image related fields to it (e.g. pull policy and cred etc). I think this approach is much easier to understand but requires more changes... if you think this is better then I'll try that.
Having image spec for server component explicitly would be more flexible for future changes (e.g what if temporal introducing other components?).
There was a problem hiding this comment.
Ok some please be more explicit on the controller behavior on the .spec.tag godoc.
Please explain that:
- It's an advanced fields if users wants to use custom builds
- If empty the operator will use the desired
.spec.version
There was a problem hiding this comment.
Where would be the most appropriate place to document the controller behavior for this field?
While adding it to the Go struct field comment is an option - since it will appear in the CRD description - this s not the place where the actual logic resides. However, including it there might still be beneficial for users who rely on the CRD documentation. Thoughts? @alexandrevilain
There was a problem hiding this comment.
Yep the Go struct field comment is the best place for me IMO, it will be available to the user through:
kubectl explain- API documentation (https://temporal-operator.pages.dev/api/v1beta1/#temporal.io/v1beta1.TemporalCluster)
|
Hi @chulkilee ! Support for temporal 1.24 has been merged and will be released soon. |
Allow overriding server / ui / admintools docker image tag, instead of assuming tag is version.