Add default/allowed ImageVersion to ConfigMap #422
Conversation
| @@ -0,0 +1,53 @@ | |||
| package v1alpha2 | |||
There was a problem hiding this comment.
Not that we have to follow it, but I quite like knative's pattern here: https://github.com/knative/serving/blob/master/pkg/apis/config/defaults.go
I see types as decoupled from the configmap. The controller may serve multiple APIs, but can only read one configmap.
There was a problem hiding this comment.
I looked through, they actually use context to pass along the default values from configmaps.
https://github.com/knative/serving/blob/master/pkg/apis/serving/v1/service_defaults.go#L31
| "0.2.3", | ||
| } | ||
| InvalidAlibiRuntimeVersionError = "RuntimeVersion must be one of " + strings.Join(AllowedAlibiRuntimeVersions, ", ") | ||
| AlibiImageName = "docker.io/seldonio/alibiexplainer" |
There was a problem hiding this comment.
If the configmap defines this, why bother even hardcoding it here?
There was a problem hiding this comment.
We can validate that the configmap and all these keys exist when starting up the controller, and crash it otherwise.
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ellis-bigelow The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/unhold |
|
/hold cancel |
|
/retest |
|
/lgtm |
* Make defaultImageVersion and allowedImageVersions configurable * Update API docs * Regenerate apidocs * Update apidocs * rebase inference service rename * Fix with InferenceService * Use inference endpoints configmap on the interface * Add framework tests * Move configmap constants * Remove hardcoded image names and default version
…tudio-kserve-storage-initializer-hermetic-poc Red Hat Konflux update kserve-storage-initializer-hermetic-poc
Revert 'Increase memory limit of kserve-controller pod'
What this PR does / why we need it:
defaultImageVersionandallowedImageVersionsto configmapWhich issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #416, #334
This change is