Skip to content

Commit

Permalink
seperate PodSpec from Service flags
Browse files Browse the repository at this point in the history
  • Loading branch information
Daisy Guo committed Jul 21, 2020
1 parent 217a813 commit c82559f
Show file tree
Hide file tree
Showing 4 changed files with 239 additions and 89 deletions.
139 changes: 51 additions & 88 deletions pkg/kn/commands/service/configuration_edit_flags.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
package service

import (
"errors"
"fmt"
"strings"

Expand All @@ -33,36 +32,24 @@ import (
)

type ConfigurationEditFlags struct {
//Fields for PodSpec
PodSpec knflags.PodFlags

// Direct field manipulation
Image uniqueStringArg
Env []string
EnvFrom []string
Mount []string
Volume []string

Command string
Arg []string

RequestsFlags, LimitsFlags ResourceFlags // TODO: Flag marked deprecated in release v0.15.0, remove in release v0.18.0
Resources knflags.ResourceOptions
Scale int
MinScale int
MaxScale int
ConcurrencyTarget int
ConcurrencyLimit int
ConcurrencyUtilization int
AutoscaleWindow string
Port string
Labels []string
LabelsService []string
LabelsRevision []string
NamePrefix string
RevisionName string
ServiceAccountName string
ImagePullSecrets string
Annotations []string
ClusterLocal bool
User int64
Scale int
MinScale int
MaxScale int
ConcurrencyTarget int
ConcurrencyLimit int
ConcurrencyUtilization int
AutoscaleWindow string
Labels []string
LabelsService []string
LabelsRevision []string
NamePrefix string
RevisionName string
Annotations []string
ClusterLocal bool

// Preferences about how to do the action.
LockToDigest bool
Expand All @@ -75,30 +62,6 @@ type ConfigurationEditFlags struct {
flags []string
}

type ResourceFlags struct {
CPU string
Memory string
}

// -- uniqueStringArg Value
// Custom implementation of flag.Value interface to prevent multiple value assignment.
// Useful to enforce unique use of flags, e.g. --image.
type uniqueStringArg string

func (s *uniqueStringArg) Set(val string) error {
if len(*s) > 0 {
return errors.New("can be provided only once")
}
*s = uniqueStringArg(val)
return nil
}

func (s *uniqueStringArg) Type() string {
return "string"
}

func (s *uniqueStringArg) String() string { return string(*s) }

// markFlagMakesRevision indicates that a flag will create a new revision if you
// set it.
func (p *ConfigurationEditFlags) markFlagMakesRevision(f string) {
Expand All @@ -107,78 +70,78 @@ func (p *ConfigurationEditFlags) markFlagMakesRevision(f string) {

// addSharedFlags adds the flags common between create & update.
func (p *ConfigurationEditFlags) addSharedFlags(command *cobra.Command) {
command.Flags().VarP(&p.Image, "image", "", "Image to run.")
command.Flags().VarP(&p.PodSpec.Image, "image", "", "Image to run.")
p.markFlagMakesRevision("image")
command.Flags().StringArrayVarP(&p.Env, "env", "e", []string{},
command.Flags().StringArrayVarP(&p.PodSpec.Env, "env", "e", []string{},
"Environment variable to set. NAME=value; you may provide this flag "+
"any number of times to set multiple environment variables. "+
"To unset, specify the environment variable name followed by a \"-\" (e.g., NAME-).")
p.markFlagMakesRevision("env")

command.Flags().StringArrayVarP(&p.EnvFrom, "env-from", "", []string{},
command.Flags().StringArrayVarP(&p.PodSpec.EnvFrom, "env-from", "", []string{},
"Add environment variables from a ConfigMap (prefix cm: or config-map:) or a Secret (prefix secret:). "+
"Example: --env-from cm:myconfigmap or --env-from secret:mysecret. "+
"You can use this flag multiple times. "+
"To unset a ConfigMap/Secret reference, append \"-\" to the name, e.g. --env-from cm:myconfigmap-.")
p.markFlagMakesRevision("env-from")

command.Flags().StringArrayVarP(&p.Mount, "mount", "", []string{},
command.Flags().StringArrayVarP(&p.PodSpec.Mount, "mount", "", []string{},
"Mount a ConfigMap (prefix cm: or config-map:), a Secret (prefix secret: or sc:), or an existing Volume (without any prefix) on the specified directory. "+
"Example: --mount /mydir=cm:myconfigmap, --mount /mydir=secret:mysecret, or --mount /mydir=myvolume. "+
"When a configmap or a secret is specified, a corresponding volume is automatically generated. "+
"You can use this flag multiple times. "+
"For unmounting a directory, append \"-\", e.g. --mount /mydir-, which also removes any auto-generated volume.")
p.markFlagMakesRevision("mount")

command.Flags().StringArrayVarP(&p.Volume, "volume", "", []string{},
command.Flags().StringArrayVarP(&p.PodSpec.Volume, "volume", "", []string{},
"Add a volume from a ConfigMap (prefix cm: or config-map:) or a Secret (prefix secret: or sc:). "+
"Example: --volume myvolume=cm:myconfigmap or --volume myvolume=secret:mysecret. "+
"You can use this flag multiple times. "+
"To unset a ConfigMap/Secret reference, append \"-\" to the name, e.g. --volume myvolume-.")
p.markFlagMakesRevision("volume")

command.Flags().StringVarP(&p.Command, "cmd", "", "",
command.Flags().StringVarP(&p.PodSpec.Command, "cmd", "", "",
"Specify command to be used as entrypoint instead of default one. "+
"Example: --cmd /app/start or --cmd /app/start --arg myArg to pass aditional arguments.")
p.markFlagMakesRevision("cmd")

command.Flags().StringArrayVarP(&p.Arg, "arg", "", []string{},
command.Flags().StringArrayVarP(&p.PodSpec.Arg, "arg", "", []string{},
"Add argument to the container command. "+
"Example: --arg myArg1 --arg --myArg2 --arg myArg3=3. "+
"You can use this flag multiple times.")
p.markFlagMakesRevision("arg")

command.Flags().StringSliceVar(&p.Resources.Limits,
command.Flags().StringSliceVar(&p.PodSpec.Resources.Limits,
"limit",
nil,
"The resource requirement limits for this Service. For example, 'cpu=100m,memory=256Mi'. "+
"You can use this flag multiple times. "+
"To unset a resource limit, append \"-\" to the resource name, e.g. '--limit memory-'.")
p.markFlagMakesRevision("limit")

command.Flags().StringSliceVar(&p.Resources.Requests,
command.Flags().StringSliceVar(&p.PodSpec.Resources.Requests,
"request",
nil,
"The resource requirement requests for this Service. For example, 'cpu=100m,memory=256Mi'. "+
"You can use this flag multiple times. "+
"To unset a resource request, append \"-\" to the resource name, e.g. '--request cpu-'.")
p.markFlagMakesRevision("request")

command.Flags().StringVar(&p.RequestsFlags.CPU, "requests-cpu", "",
command.Flags().StringVar(&p.PodSpec.RequestsFlags.CPU, "requests-cpu", "",
"DEPRECATED: please use --request instead. The requested CPU (e.g., 250m).")
p.markFlagMakesRevision("requests-cpu")

command.Flags().StringVar(&p.RequestsFlags.Memory, "requests-memory", "",
command.Flags().StringVar(&p.PodSpec.RequestsFlags.Memory, "requests-memory", "",
"DEPRECATED: please use --request instead. The requested memory (e.g., 64Mi).")
p.markFlagMakesRevision("requests-memory")

// TODO: Flag marked deprecated in release v0.15.0, remove in release v0.18.0
command.Flags().StringVar(&p.LimitsFlags.CPU, "limits-cpu", "",
command.Flags().StringVar(&p.PodSpec.LimitsFlags.CPU, "limits-cpu", "",
"DEPRECATED: please use --limit instead. The limits on the requested CPU (e.g., 1000m).")
p.markFlagMakesRevision("limits-cpu")

// TODO: Flag marked deprecated in release v0.15.0, remove in release v0.18.0
command.Flags().StringVar(&p.LimitsFlags.Memory, "limits-memory", "",
command.Flags().StringVar(&p.PodSpec.LimitsFlags.Memory, "limits-memory", "",
"DEPRECATED: please use --limit instead. The limits on the requested memory (e.g., 1024Mi).")
p.markFlagMakesRevision("limits-memory")

Expand Down Expand Up @@ -213,7 +176,7 @@ func (p *ConfigurationEditFlags) addSharedFlags(command *cobra.Command) {
"Percentage of concurrent requests utilization before scaling up.")
p.markFlagMakesRevision("concurrency-utilization")

command.Flags().StringVarP(&p.Port, "port", "p", "", "The port where application listens on, in the format 'NAME:PORT', where 'NAME' is optional. Examples: '--port h2c:8080' , '--port 8080'.")
command.Flags().StringVarP(&p.PodSpec.Port, "port", "p", "", "The port where application listens on, in the format 'NAME:PORT', where 'NAME' is optional. Examples: '--port h2c:8080' , '--port 8080'.")
p.markFlagMakesRevision("port")

command.Flags().StringArrayVarP(&p.Labels, "label", "l", []string{},
Expand Down Expand Up @@ -247,7 +210,7 @@ func (p *ConfigurationEditFlags) addSharedFlags(command *cobra.Command) {
"the image. (--no-lock-to-digest pulls the image tag afresh with each new revision)")
// Don't mark as changing the revision.

command.Flags().StringVar(&p.ServiceAccountName,
command.Flags().StringVar(&p.PodSpec.ServiceAccountName,
"service-account",
"",
"Service account name to set. An empty argument (\"\") clears the service account. The referenced service account must exist in the service's namespace.")
Expand All @@ -259,12 +222,12 @@ func (p *ConfigurationEditFlags) addSharedFlags(command *cobra.Command) {
"To unset, specify the annotation name followed by a \"-\" (e.g., name-).")
p.markFlagMakesRevision("annotation")

command.Flags().StringVar(&p.ImagePullSecrets,
command.Flags().StringVar(&p.PodSpec.ImagePullSecrets,
"pull-secret",
"",
"Image pull secret to set. An empty argument (\"\") clears the pull secret. The referenced secret must exist in the service's namespace.")
p.markFlagMakesRevision("pull-secret")
command.Flags().Int64VarP(&p.User, "user", "", 0, "The user ID to run the container (e.g., 1001).")
command.Flags().Int64VarP(&p.PodSpec.User, "user", "", 0, "The user ID to run the container (e.g., 1001).")
p.markFlagMakesRevision("user")
}

Expand All @@ -290,7 +253,7 @@ func (p *ConfigurationEditFlags) Apply(

template := &service.Spec.Template
if cmd.Flags().Changed("env") {
envMap, err := util.MapFromArrayAllowingSingles(p.Env, "=")
envMap, err := util.MapFromArrayAllowingSingles(p.PodSpec.Env, "=")
if err != nil {
return fmt.Errorf("Invalid --env: %w", err)
}
Expand All @@ -305,7 +268,7 @@ func (p *ConfigurationEditFlags) Apply(
if cmd.Flags().Changed("env-from") {
envFromSourceToUpdate := []string{}
envFromSourceToRemove := []string{}
for _, name := range p.EnvFrom {
for _, name := range p.PodSpec.EnvFrom {
if name == "-" {
return fmt.Errorf("\"-\" is not a valid value for \"--env-from\"")
} else if strings.HasSuffix(name, "-") {
Expand All @@ -322,12 +285,12 @@ func (p *ConfigurationEditFlags) Apply(
}

if cmd.Flags().Changed("mount") || cmd.Flags().Changed("volume") {
mountsToUpdate, mountsToRemove, err := util.OrderedMapAndRemovalListFromArray(p.Mount, "=")
mountsToUpdate, mountsToRemove, err := util.OrderedMapAndRemovalListFromArray(p.PodSpec.Mount, "=")
if err != nil {
return fmt.Errorf("Invalid --mount: %w", err)
}

volumesToUpdate, volumesToRemove, err := util.OrderedMapAndRemovalListFromArray(p.Volume, "=")
volumesToUpdate, volumesToRemove, err := util.OrderedMapAndRemovalListFromArray(p.PodSpec.Volume, "=")
if err != nil {
return fmt.Errorf("Invalid --volume: %w", err)
}
Expand All @@ -349,7 +312,7 @@ func (p *ConfigurationEditFlags) Apply(

imageSet := false
if cmd.Flags().Changed("image") {
err = servinglib.UpdateImage(template, p.Image.String())
err = servinglib.UpdateImage(template, p.PodSpec.Image.String())
if err != nil {
return err
}
Expand Down Expand Up @@ -383,11 +346,11 @@ func (p *ConfigurationEditFlags) Apply(
fmt.Fprintf(cmd.OutOrStdout(), "\nWARNING: flags --requests-cpu / --requests-memory are deprecated and going to be removed in future release, please use --request instead.\n\n")
}

limitsResources, err := p.computeResources(p.LimitsFlags)
limitsResources, err := p.computeResources(p.PodSpec.LimitsFlags)
if err != nil {
return err
}
requestsResources, err := p.computeResources(p.RequestsFlags)
requestsResources, err := p.computeResources(p.PodSpec.RequestsFlags)
if err != nil {
return err
}
Expand All @@ -396,32 +359,32 @@ func (p *ConfigurationEditFlags) Apply(
return err
}

requestsToRemove, limitsToRemove, err := p.Resources.Validate()
requestsToRemove, limitsToRemove, err := p.PodSpec.Resources.Validate()
if err != nil {
return err
}

err = servinglib.UpdateResources(template, p.Resources.ResourceRequirements, requestsToRemove, limitsToRemove)
err = servinglib.UpdateResources(template, p.PodSpec.Resources.ResourceRequirements, requestsToRemove, limitsToRemove)
if err != nil {
return err
}

if cmd.Flags().Changed("cmd") {
err = servinglib.UpdateContainerCommand(template, p.Command)
err = servinglib.UpdateContainerCommand(template, p.PodSpec.Command)
if err != nil {
return err
}
}

if cmd.Flags().Changed("arg") {
err = servinglib.UpdateContainerArg(template, p.Arg)
err = servinglib.UpdateContainerArg(template, p.PodSpec.Arg)
if err != nil {
return err
}
}

if cmd.Flags().Changed("port") {
err = servinglib.UpdateContainerPort(template, p.Port)
err = servinglib.UpdateContainerPort(template, p.PodSpec.Port)
if err != nil {
return err
}
Expand Down Expand Up @@ -527,18 +490,18 @@ func (p *ConfigurationEditFlags) Apply(
}

if cmd.Flags().Changed("service-account") {
err = servinglib.UpdateServiceAccountName(template, p.ServiceAccountName)
err = servinglib.UpdateServiceAccountName(template, p.PodSpec.ServiceAccountName)
if err != nil {
return err
}
}

if cmd.Flags().Changed("pull-secret") {
servinglib.UpdateImagePullSecrets(template, p.ImagePullSecrets)
servinglib.UpdateImagePullSecrets(template, p.PodSpec.ImagePullSecrets)
}

if cmd.Flags().Changed("user") {
servinglib.UpdateUser(template, p.User)
servinglib.UpdateUser(template, p.PodSpec.User)
}

return nil
Expand All @@ -558,7 +521,7 @@ func (p *ConfigurationEditFlags) updateLabels(obj *metav1.ObjectMeta, flagLabels
return nil
}

func (p *ConfigurationEditFlags) computeResources(resourceFlags ResourceFlags) (corev1.ResourceList, error) {
func (p *ConfigurationEditFlags) computeResources(resourceFlags knflags.ResourceFlags) (corev1.ResourceList, error) {
resourceList := corev1.ResourceList{}

if resourceFlags.CPU != "" {
Expand All @@ -584,7 +547,7 @@ func (p *ConfigurationEditFlags) computeResources(resourceFlags ResourceFlags) (
return resourceList, nil
}

// AnyMutation returns true if there are any revision template mutations in the
// AnyMutation returns true if there are any revision template corev1 "k8s.io/api/core/v1" in the
// command.
func (p *ConfigurationEditFlags) AnyMutation(cmd *cobra.Command) bool {
for _, flag := range p.flags {
Expand Down
2 changes: 1 addition & 1 deletion pkg/kn/commands/service/create.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ func NewServiceCreateCommand(p *commands.KnParams) *cobra.Command {
if len(args) == 1 {
name = args[0]
}
if editFlags.Image == "" && editFlags.Filename == "" {
if editFlags.PodSpec.Image == "" && editFlags.Filename == "" {
return errors.New("'service create' requires the image name to run provided with the --image option")
}

Expand Down
Loading

0 comments on commit c82559f

Please sign in to comment.