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

Add resource generic load, unload, status to CLI #4660

Merged
merged 2 commits into from
Feb 14, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 63 additions & 0 deletions operator/cmd/seldon/cli/load.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
package cli

import (
"k8s.io/utils/env"

"github.com/seldonio/seldon-core/operator/v2/pkg/cli"
"github.com/spf13/cobra"
)

func createLoad() *cobra.Command {
cmd := &cobra.Command{
Use: "load",
Short: "load resources",
Long: `load resources`,
Args: cobra.MinimumNArgs(0),
RunE: func(cmd *cobra.Command, args []string) error {
flags := cmd.Flags()

schedulerHostIsSet := flags.Changed(flagSchedulerHost)
schedulerHost, err := flags.GetString(flagSchedulerHost)
if err != nil {
return err
}
authority, err := flags.GetString(flagAuthority)
if err != nil {
return err
}
filename, err := flags.GetString(flagFile)
if err != nil {
return err
}
showRequest, err := flags.GetBool(flagShowRequest)
if err != nil {
return err
}
showResponse, err := flags.GetBool(flagShowResponse)
if err != nil {
return err
}

schedulerClient, err := cli.NewSchedulerClient(schedulerHost, schedulerHostIsSet, authority)
if err != nil {
return err
}

var dataFile []byte
if filename != "" {
dataFile = loadFile(filename)
}
err = schedulerClient.Load(dataFile, showRequest, showResponse)
return err
},
}

flags := cmd.Flags()
flags.BoolP(flagShowRequest, "r", false, "show request")
flags.BoolP(flagShowResponse, "o", false, "show response")
flags.String(flagSchedulerHost, env.GetString(envScheduler, defaultSchedulerHost), helpSchedulerHost)
flags.String(flagAuthority, "", helpAuthority)
flags.StringP(flagFile, "f", "", "model manifest file (YAML)")

return cmd
}
7 changes: 6 additions & 1 deletion operator/cmd/seldon/cli/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,16 @@ func GetCmd() *cobra.Command {
cmdConfigRemove := createConfigRemove()
cmdConfigList := createConfigList()

// Generic commands
cmdLoad := createLoad()
cmdUnload := createUnload()
cmdStatus := createStatus()

var rootCmd = &cobra.Command{Use: "seldon", SilenceErrors: false, SilenceUsage: true}

rootCmd.DisableAutoGenTag = true

rootCmd.AddCommand(cmdModel, cmdServer, cmdExperiment, cmdPipeline, cmdConfig)
rootCmd.AddCommand(cmdModel, cmdServer, cmdExperiment, cmdPipeline, cmdConfig, cmdLoad, cmdUnload, cmdStatus)
Copy link
Member

Choose a reason for hiding this comment

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

just to keep consistency with previous usage (i.e. seldon model load and seldon pipeline load), an alternative is perhaps to use
seldon all load? I am personally not entirely sure which approach is better so happy to leave it as is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

seems a bit verbose - maybe will leave for now

cmdModel.AddCommand(cmdModelLoad, cmdModelUnload, cmdModelStatus, cmdModelInfer, cmdModelMeta, cmdModelList)
cmdServer.AddCommand(cmdServerStatus, cmdServerList)
cmdExperiment.AddCommand(cmdExperimentStart, cmdExperimentStop, cmdExperimentStatus, cmdExperimentList)
Expand Down
67 changes: 67 additions & 0 deletions operator/cmd/seldon/cli/status.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
package cli

import (
"k8s.io/utils/env"

"github.com/seldonio/seldon-core/operator/v2/pkg/cli"
"github.com/spf13/cobra"
)

func createStatus() *cobra.Command {
cmd := &cobra.Command{
Use: "status <pipelineName>",
Short: "status of a pipeline",
Long: `status of a pipeline`,
Args: cobra.ExactArgs(0),
RunE: func(cmd *cobra.Command, args []string) error {
flags := cmd.Flags()

schedulerHostIsSet := flags.Changed(flagSchedulerHost)
schedulerHost, err := flags.GetString(flagSchedulerHost)
if err != nil {
return err
}
authority, err := flags.GetString(flagAuthority)
if err != nil {
return err
}
showRequest, err := flags.GetBool(flagShowRequest)
if err != nil {
return err
}
showResponse, err := flags.GetBool(flagShowResponse)
if err != nil {
return err
}
waitCondition, err := flags.GetBool(flagWaitCondition)
if err != nil {
return err
}
filename, err := flags.GetString(flagFile)
if err != nil {
return err
}
var dataFile []byte
if filename != "" {
dataFile = loadFile(filename)
}
schedulerClient, err := cli.NewSchedulerClient(schedulerHost, schedulerHostIsSet, authority)
if err != nil {
return err
}

err = schedulerClient.Status(dataFile, showRequest, showResponse, waitCondition)
return err
},
}

flags := cmd.Flags()
flags.BoolP(flagShowRequest, "r", false, "show request")
flags.BoolP(flagShowResponse, "o", false, "show response")
flags.String(flagSchedulerHost, env.GetString(envScheduler, defaultSchedulerHost), helpSchedulerHost)
flags.String(flagAuthority, "", helpAuthority)
flags.BoolP(flagWaitCondition, "w", false, "wait for resources to be ready")
flags.StringP(flagFile, "f", "", "model manifest file (YAML)")

return cmd
}
62 changes: 62 additions & 0 deletions operator/cmd/seldon/cli/unload.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package cli

import (
"k8s.io/utils/env"

"github.com/seldonio/seldon-core/operator/v2/pkg/cli"
"github.com/spf13/cobra"
)

func createUnload() *cobra.Command {
cmd := &cobra.Command{
Use: "unload resources",
Short: "unload resources",
Long: `unload resources`,
RunE: func(cmd *cobra.Command, args []string) error {
flags := cmd.Flags()

schedulerHostIsSet := flags.Changed(flagSchedulerHost)
schedulerHost, err := flags.GetString(flagSchedulerHost)
if err != nil {
return err
}
authority, err := flags.GetString(flagAuthority)
if err != nil {
return err
}
showRequest, err := flags.GetBool(flagShowRequest)
if err != nil {
return err
}
showResponse, err := flags.GetBool(flagShowResponse)
if err != nil {
return err
}
filename, err := flags.GetString(flagFile)
if err != nil {
return err
}
var dataFile []byte
if filename != "" {
dataFile = loadFile(filename)
}

schedulerClient, err := cli.NewSchedulerClient(schedulerHost, schedulerHostIsSet, authority)
if err != nil {
return err
}

err = schedulerClient.Unload(dataFile, showRequest, showResponse)
sakoush marked this conversation as resolved.
Show resolved Hide resolved
return err
},
}

flags := cmd.Flags()
flags.BoolP(flagShowRequest, "r", false, "show request")
flags.BoolP(flagShowResponse, "o", false, "show response")
flags.String(flagSchedulerHost, env.GetString(envScheduler, defaultSchedulerHost), helpSchedulerHost)
flags.String(flagAuthority, "", helpAuthority)
flags.StringP(flagFile, "f", "", "model manifest file (YAML)")

return cmd
}
109 changes: 109 additions & 0 deletions operator/pkg/cli/generic.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
package cli

import (
"bytes"
"errors"
"io"
"os"

"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
yaml2 "k8s.io/apimachinery/pkg/util/yaml"
Copy link
Member

Choose a reason for hiding this comment

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

does it have to be yaml2?

)

func createDecoder(data []byte) *yaml2.YAMLOrJSONDecoder {
var reader io.Reader
if len(data) > 0 {
reader = bytes.NewReader(data)
} else {
reader = io.Reader(os.Stdin)
}
dec := yaml2.NewYAMLOrJSONDecoder(reader, 10)
return dec
}

func getNextResource(dec *yaml2.YAMLOrJSONDecoder) (string, string, []byte, bool, error) {
Copy link
Member

Choose a reason for hiding this comment

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

too many returns and it is hard to know what we are getting back. Consider adding at least description.

unstructuredObject := &unstructured.Unstructured{}
if err := dec.Decode(unstructuredObject); err != nil {
if errors.Is(err, io.EOF) {
return "", "", nil, false, nil
}
return "", "", nil, false, err
}
// Get the resource kind nad original bytes
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// Get the resource kind nad original bytes
// Get the resource kind and original bytes

gvk := unstructuredObject.GroupVersionKind()
data, err := unstructuredObject.MarshalJSON()
if err != nil {
return "", "", nil, false, err
}
return gvk.Kind, unstructuredObject.GetName(), data, true, nil
}

func (sc *SchedulerClient) Load(data []byte, showRequest bool, showResponse bool) error {
dec := createDecoder(data)
for {
kind, _, data, keepGoing, err := getNextResource(dec)
Copy link
Member

Choose a reason for hiding this comment

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

we are also not checking err before the switch

if !keepGoing {
return err
}
switch kind {
Copy link
Member

Choose a reason for hiding this comment

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

should we add a warning if it is not part of these 3 resources? should we stop?

case "Model":
err = sc.LoadModel(data, showRequest, showResponse)
case "Pipeline":
err = sc.LoadPipeline(data, showRequest, showResponse)
case "Experiment":
err = sc.StartExperiment(data, showRequest, showResponse)
}
if err != nil {
return err
}
}
}

func (sc *SchedulerClient) Unload(data []byte, showRequest bool, showResponse bool) error {
dec := createDecoder(data)
for {
kind, _, data, keepGoing, err := getNextResource(dec)
Copy link
Member

Choose a reason for hiding this comment

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

ditto

if !keepGoing {
return err
}
switch kind {
case "Model":
err = sc.UnloadModel("", data, showRequest, showResponse)
case "Pipeline":
err = sc.UnloadPipeline("", data, showRequest, showResponse)
case "Experiment":
err = sc.StopExperiment("", data, showRequest, showResponse)
}
if err != nil {
return err
}
}
}

func (sc *SchedulerClient) Status(data []byte, showRequest bool, showResponse bool, wait bool) error {
dec := createDecoder(data)
for {
kind, name, _, keepGoing, err := getNextResource(dec)
if !keepGoing {
return err
}
waitCondition := ""
switch kind {
case "Model":
Copy link
Member

Choose a reason for hiding this comment

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

consider extracting the values of the switch into enum as it is used in 3 different places already.

if wait {
waitCondition = "ModelAvailable"
}
err = sc.ModelStatus(name, showRequest, showResponse, waitCondition)
case "Pipeline":
if wait {
waitCondition = "PipelineReady"
}
err = sc.PipelineStatus(name, showRequest, showResponse, waitCondition)
case "Experiment":
err = sc.ExperimentStatus(name, showRequest, showResponse, wait)
Copy link
Member

Choose a reason for hiding this comment

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

no wait on Experiments?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the bool is passed directly to function - experiments don't take a string waitCondition

}
if err != nil {
return err
}
}
}
19 changes: 2 additions & 17 deletions operator/pkg/cli/scheduler.go
Original file line number Diff line number Diff line change
Expand Up @@ -283,14 +283,8 @@ func (sc *SchedulerClient) ModelStatus(modelName string, showRequest bool, showR
return err
}
}
if !showResponse {
if len(res.Versions) > 0 {
modelStatus := res.Versions[0].State.GetState().String()
fmt.Printf("{\"%s\":\"%s\"}\n", modelName, modelStatus)
} else {
fmt.Println("Unknown")
}
} else {

if showResponse {
printProto(res)
}
return nil
Expand Down Expand Up @@ -540,8 +534,6 @@ func (sc *SchedulerClient) ExperimentStatus(experimentName string, showRequest b
}
if showResponse {
printProto(res)
} else {
fmt.Printf("%v", res.Active)
}
return nil
}
Expand Down Expand Up @@ -715,13 +707,6 @@ func (sc *SchedulerClient) PipelineStatus(pipelineName string, showRequest bool,
}
if showResponse {
printProto(res)
} else {
if len(res.Versions) > 0 {
fmt.Printf("%v", res.Versions[0].State.Status.String())
} else {
fmt.Println("Unknown status")
}

}
return nil
}
Expand Down