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

feat: Add accessmode, custom image, and image secret #98

Merged
merged 6 commits into from
Oct 25, 2023

Conversation

ishaansehgal99
Copy link
Collaborator

@ishaansehgal99 ishaansehgal99 commented Oct 24, 2023

set the custom image, image pull secrets, and access mode for the inference object. This would allow for pulling from a custom image.

@ishaansehgal99 ishaansehgal99 changed the title feat: add dynamic setting of inference object feat: Add accessmode, image, and image secret to KAITO Oct 24, 2023
@ishaansehgal99 ishaansehgal99 changed the title feat: Add accessmode, image, and image secret to KAITO feat: Add accessmode, custom image, and image secret Oct 24, 2023
@ishaansehgal99 ishaansehgal99 marked this pull request as ready for review October 25, 2023 00:37
Copy link
Collaborator

@Fei-Guo Fei-Guo left a comment

Choose a reason for hiding this comment

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

registryName = os.Getenv("PRESET_REGISTRY_NAME")

We need to remove it from chart and code. We have formal API support to provide image ur.

@@ -80,5 +84,10 @@ func (i *InferenceSpec) validateUpdate(old *InferenceSpec) (errs *apis.FieldErro
if (i.Template != nil && old.Template == nil) || (i.Template == nil && old.Template != nil) {
errs = errs.Also(apis.ErrGeneric("field cannot be unset/set if it was set/unset", "template"))
}

if i.Preset.PresetMeta.AccessMode == "private" && i.Preset.PresetOptions.Image == "" {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This check is not needed for update case. Update always means old compares with new.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes and I believe this check ensures image isn't changed:

!reflect.DeepEqual(i.Preset, old.Preset)

default:
err := fmt.Errorf("preset model %s is not supported", presetName)
return inference.PresetInferenceParam{}, err
}

inferenceObj.AccessMode = string(wObj.Inference.Preset.PresetMeta.AccessMode)
if wObj.Inference.Preset.PresetOptions.Image != "" {
Copy link
Collaborator

Choose a reason for hiding this comment

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

You need to check access mode here as well.

@Fei-Guo Fei-Guo merged commit 64c7a89 into main Oct 25, 2023
5 checks passed
@Fei-Guo Fei-Guo deleted the Ishaan/private-registry-url branch October 25, 2023 20:56
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