Skip to content

Commit

Permalink
Add ResolvePodSpec to podspec.go and move the related utilities to po…
Browse files Browse the repository at this point in the history 
…dspec_helper (#1024)

* moving utilities of handling podspec to podspec_helper

* address comments

* refactor code base

* change the input parameter from ccmd to flagset

* remove comments and add CHANGELOG
  • Loading branch information
Ying Chun Guo authored Oct 27, 2020
1 parent abb75e4 commit ed67482
Show file tree
Hide file tree
Showing 14 changed files with 1,417 additions and 1,167 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.adoc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@
|===
| | Description | PR

| 🐣
| Add function ResolvePodSpec and move the utilities to podspec_helper
| https://github.com/knative/client/pull/1024[#1024]

| 🐛
| Fix various misspellings and linter items
| https://github.com/knative/client/pull/1057[#1057]
Expand Down
140 changes: 3 additions & 137 deletions pkg/kn/commands/service/configuration_edit_flags.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ import (

"github.com/spf13/cobra"

corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

knflags "knative.dev/client/pkg/kn/flags"
Expand Down Expand Up @@ -198,53 +196,10 @@ func (p *ConfigurationEditFlags) Apply(
cmd *cobra.Command) error {

template := &service.Spec.Template
if cmd.Flags().Changed("env") {
envMap, err := util.MapFromArrayAllowingSingles(p.PodSpecFlags.Env, "=")
if err != nil {
return fmt.Errorf("Invalid --env: %w", err)
}

envToRemove := util.ParseMinusSuffix(envMap)
err = servinglib.UpdateEnvVars(template, envMap, envToRemove)
if err != nil {
return err
}
}

if cmd.Flags().Changed("env-from") {
envFromSourceToUpdate := []string{}
envFromSourceToRemove := []string{}
for _, name := range p.PodSpecFlags.EnvFrom {
if name == "-" {
return fmt.Errorf("\"-\" is not a valid value for \"--env-from\"")
} else if strings.HasSuffix(name, "-") {
envFromSourceToRemove = append(envFromSourceToRemove, name[:len(name)-1])
} else {
envFromSourceToUpdate = append(envFromSourceToUpdate, name)
}
}

err := servinglib.UpdateEnvFrom(template, envFromSourceToUpdate, envFromSourceToRemove)
if err != nil {
return err
}
}

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

volumesToUpdate, volumesToRemove, err := util.OrderedMapAndRemovalListFromArray(p.PodSpecFlags.Volume, "=")
if err != nil {
return fmt.Errorf("Invalid --volume: %w", err)
}

err = servinglib.UpdateVolumeMountsAndVolumes(template, mountsToUpdate, mountsToRemove, volumesToUpdate, volumesToRemove)
if err != nil {
return err
}
err := p.PodSpecFlags.ResolvePodSpec(&template.Spec.PodSpec, cmd.Flags())
if err != nil {
return err
}

name, err := servinglib.GenerateRevisionName(p.RevisionName, service)
Expand All @@ -258,10 +213,6 @@ func (p *ConfigurationEditFlags) Apply(

imageSet := false
if cmd.Flags().Changed("image") {
err = servinglib.UpdateImage(template, p.PodSpecFlags.Image.String())
if err != nil {
return err
}
imageSet = true
}
_, userImagePresent := template.Annotations[servinglib.UserImageAnnotationKey]
Expand Down Expand Up @@ -292,50 +243,6 @@ 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.PodSpecFlags.LimitsFlags)
if err != nil {
return err
}
requestsResources, err := p.computeResources(p.PodSpecFlags.RequestsFlags)
if err != nil {
return err
}
err = servinglib.UpdateResourcesDeprecated(template, requestsResources, limitsResources)
if err != nil {
return err
}

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

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

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

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

if cmd.Flags().Changed("port") {
err = servinglib.UpdateContainerPort(template, p.PodSpecFlags.Port)
if err != nil {
return err
}
}

// Deprecated "min-scale" in 0.19, updated to "scale-min"
if cmd.Flags().Changed("scale-min") || cmd.Flags().Changed("min-scale") {
err = servinglib.UpdateMinScale(template, p.MinScale)
Expand Down Expand Up @@ -472,21 +379,6 @@ func (p *ConfigurationEditFlags) Apply(

}

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

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

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

if cmd.Flags().Changed("scale-init") {
containsAnnotation := func(annotationList []string, annotation string) bool {
for _, element := range annotationList {
Expand Down Expand Up @@ -524,32 +416,6 @@ func (p *ConfigurationEditFlags) updateLabels(obj *metav1.ObjectMeta, flagLabels
return nil
}

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

if resourceFlags.CPU != "" {
cpuQuantity, err := resource.ParseQuantity(resourceFlags.CPU)
if err != nil {
return corev1.ResourceList{},
fmt.Errorf("Error parsing %q: %w", resourceFlags.CPU, err)
}

resourceList[corev1.ResourceCPU] = cpuQuantity
}

if resourceFlags.Memory != "" {
memoryQuantity, err := resource.ParseQuantity(resourceFlags.Memory)
if err != nil {
return corev1.ResourceList{},
fmt.Errorf("Error parsing %q: %w", resourceFlags.Memory, err)
}

resourceList[corev1.ResourceMemory] = memoryQuantity
}

return resourceList, nil
}

// AnyMutation returns true if there are any revision template mutations in the
// command.
func (p *ConfigurationEditFlags) AnyMutation(cmd *cobra.Command) bool {
Expand Down
8 changes: 4 additions & 4 deletions pkg/kn/commands/service/create_mock_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ func TestServiceCreateWithMountConfigMap(t *testing.T) {
template := &service.Spec.Template
template.Spec.Volumes = []corev1.Volume{
{
Name: servinglib.GenerateVolumeName("/mount/path"),
Name: util.GenerateVolumeName("/mount/path"),
VolumeSource: corev1.VolumeSource{
ConfigMap: &corev1.ConfigMapVolumeSource{
LocalObjectReference: corev1.LocalObjectReference{
Expand All @@ -292,7 +292,7 @@ func TestServiceCreateWithMountConfigMap(t *testing.T) {

template.Spec.Containers[0].VolumeMounts = []corev1.VolumeMount{
{
Name: servinglib.GenerateVolumeName("/mount/path"),
Name: util.GenerateVolumeName("/mount/path"),
MountPath: "/mount/path",
ReadOnly: true,
},
Expand Down Expand Up @@ -359,7 +359,7 @@ func TestServiceCreateWithMountSecret(t *testing.T) {
template := &service.Spec.Template
template.Spec.Volumes = []corev1.Volume{
{
Name: servinglib.GenerateVolumeName("/mount/path"),
Name: util.GenerateVolumeName("/mount/path"),
VolumeSource: corev1.VolumeSource{
Secret: &corev1.SecretVolumeSource{
SecretName: "secret-name",
Expand All @@ -370,7 +370,7 @@ func TestServiceCreateWithMountSecret(t *testing.T) {

template.Spec.Containers[0].VolumeMounts = []corev1.VolumeMount{
{
Name: servinglib.GenerateVolumeName("/mount/path"),
Name: util.GenerateVolumeName("/mount/path"),
MountPath: "/mount/path",
ReadOnly: true,
},
Expand Down
11 changes: 5 additions & 6 deletions pkg/kn/commands/service/create_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import (
"k8s.io/apimachinery/pkg/watch"

"knative.dev/client/pkg/kn/commands"
servinglib "knative.dev/client/pkg/serving"
"knative.dev/client/pkg/util"
"knative.dev/client/pkg/wait"
network "knative.dev/networking/pkg"
Expand Down Expand Up @@ -225,7 +224,7 @@ func TestServiceCreateEnv(t *testing.T) {
if err != nil {
t.Fatal(err)
}
actualEnvVars, err := servinglib.EnvToMap(template.Spec.Containers[0].Env)
actualEnvVars, err := util.EnvToMap(template.Spec.Containers[0].Env)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -726,7 +725,7 @@ func TestServiceCreateEnvForce(t *testing.T) {
if err != nil {
t.Fatal(err)
}
actualEnvVars, err := servinglib.EnvToMap(template.Spec.Containers[0].Env)
actualEnvVars, err := util.EnvToMap(template.Spec.Containers[0].Env)
if err != nil {
t.Fatal(err)
} else if template.Spec.Containers[0].Image != "gcr.io/foo/bar:v2" {
Expand Down Expand Up @@ -993,7 +992,7 @@ func TestServiceCreateFromYAMLWithOverride(t *testing.T) {
assert.Assert(t, action.Matches("create", "services"))
assert.Equal(t, created.Name, "foo")

actualEnvVar, err := servinglib.EnvToMap(created.Spec.Template.Spec.GetContainer().Env)
actualEnvVar, err := util.EnvToMap(created.Spec.Template.Spec.GetContainer().Env)
assert.NilError(t, err)
assert.DeepEqual(t, actualEnvVar, expectedEnvVars)

Expand All @@ -1007,7 +1006,7 @@ func TestServiceCreateFromYAMLWithOverride(t *testing.T) {
assert.Assert(t, action.Matches("create", "services"))
assert.Equal(t, created.Name, "foo")

actualEnvVar, err = servinglib.EnvToMap(created.Spec.Template.Spec.GetContainer().Env)
actualEnvVar, err = util.EnvToMap(created.Spec.Template.Spec.GetContainer().Env)
assert.NilError(t, err)
assert.DeepEqual(t, actualEnvVar, expectedEnvVars)

Expand All @@ -1020,7 +1019,7 @@ func TestServiceCreateFromYAMLWithOverride(t *testing.T) {
assert.Assert(t, action.Matches("create", "services"))
assert.Equal(t, created.Name, "foo")

actualEnvVar, err = servinglib.EnvToMap(created.Spec.Template.Spec.GetContainer().Env)
actualEnvVar, err = util.EnvToMap(created.Spec.Template.Spec.GetContainer().Env)
assert.NilError(t, err)
assert.DeepEqual(t, actualEnvVar, expectedEnvVars)

Expand Down
Loading

0 comments on commit ed67482

Please sign in to comment.