knative
/
client
mirror of https://github.com/knative/client.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix(service update): Only update fields which have been sent by server. (#155) 

* fix(service update): Only update fields which have been sent by server.

This reflects the lemonade process step1. Tests have been adapted to
verify this behaviours.

The only situation when we update field coming from the server is for
"kn service update" for envs, image and requests/limits.

All other operation are either create (here, we always have to send the
old fields), or read (get/describe).

Fixes #144.

* chore: typo fix

* refactor(service update/create): Moved from Configuration to RevisionTemplateSpec

In order to proper handling the v1alpha1 -> v1beta1 migration methods has been updated to get rid fo Configuration within the service as this
is completely inlined in v1beta1.

The helper methods have been also updated accordingly.
I think we are good now.
This commit is contained in:
Roland Huß 2019-06-03 22:30:35 +02:00 committed by Knative Prow Robot
parent 43061af74a
commit b6a8fa9213
8 changed files with 261 additions and 140 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
pkg/kn/commands/configuration_edit_flags.go
View File

@ -54,7 +54,13 @@ func (p *ConfigurationEditFlags) AddCreateFlags(command *cobra.Command) {
command.MarkFlagRequired("image")
}
func (p *ConfigurationEditFlags) Apply(config *servingv1alpha1.ConfigurationSpec, cmd *cobra.Command) error {
func (p *ConfigurationEditFlags) Apply(service *servingv1alpha1.Service, cmd *cobra.Command) error {
template, err := servinglib.GetRevisionTemplate(service)
if err != nil {
return err
}
envMap := map[string]string{}
for _, pairStr := range p.Env {
pairSlice := strings.SplitN(pairStr, "=", 2)
@ -65,12 +71,12 @@ func (p *ConfigurationEditFlags) Apply(config *servingv1alpha1.ConfigurationSpec
}
envMap[pairSlice[0]] = pairSlice[1]
}
err := servinglib.UpdateEnvVars(config, envMap)
if err != nil {
if err := servinglib.UpdateEnvVars(template, envMap); err != nil {
return err
}
if cmd.Flags().Changed("image") {
err = servinglib.UpdateImage(config, p.Image)
err = servinglib.UpdateImage(template, p.Image)
if err != nil {
return err
}
@ -83,7 +89,7 @@ func (p *ConfigurationEditFlags) Apply(config *servingv1alpha1.ConfigurationSpec
if err != nil {
return err
}
err = servinglib.UpdateResources(config, requestsResources, limitsResources)
err = servinglib.UpdateResources(template, requestsResources, limitsResources)
if err != nil {
return err
}

7
pkg/kn/commands/service_create.go
View File

@ -18,7 +18,6 @@ import (
"errors"
"fmt"
serving_lib "github.com/knative/client/pkg/serving"
servingv1alpha1 "github.com/knative/serving/pkg/apis/serving/v1alpha1"
"github.com/spf13/cobra"
corev1 "k8s.io/api/core/v1"
@ -81,11 +80,7 @@ func NewServiceCreateCommand(p *KnParams) *cobra.Command {
},
}
config, err := serving_lib.GetConfiguration(&service)
if err != nil {
return err
}
err = editFlags.Apply(config, cmd)
err = editFlags.Apply(&service, cmd)
if err != nil {
return err
}

74
pkg/kn/commands/service_create_test.go
View File

@ -74,11 +74,11 @@ func TestServiceCreateImage(t *testing.T) {
} else if !action.Matches("create", "services") {
t.Fatalf("Bad action %v", action)
}
conf, err := servinglib.GetConfiguration(created)
template, err := servinglib.GetRevisionTemplate(created)
if err != nil {
t.Fatal(err)
} else if conf.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Image != "gcr.io/foo/bar:baz" {
t.Fatalf("wrong image set: %v", conf.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Image)
} else if template.Spec.DeprecatedContainer.Image != "gcr.io/foo/bar:baz" {
t.Fatalf("wrong image set: %v", template.Spec.DeprecatedContainer.Image)
} else if !strings.Contains(output, "foo") || !strings.Contains(output, "created") ||
!strings.Contains(output, "default") {
t.Fatalf("wrong stdout message: %v", output)
@ -99,20 +99,20 @@ func TestServiceCreateEnv(t *testing.T) {
"A": "DOGS",
"B": "WOLVES"}
conf, err := servinglib.GetConfiguration(created)
actualEnvVars, err := servinglib.EnvToMap(conf.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Env)
template, err := servinglib.GetRevisionTemplate(created)
actualEnvVars, err := servinglib.EnvToMap(template.Spec.DeprecatedContainer.Env)
if err != nil {
t.Fatal(err)
}
if err != nil {
t.Fatal(err)
} else if conf.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Image != "gcr.io/foo/bar:baz" {
t.Fatalf("wrong image set: %v", conf.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Image)
} else if template.Spec.DeprecatedContainer.Image != "gcr.io/foo/bar:baz" {
t.Fatalf("wrong image set: %v", template.Spec.DeprecatedContainer.Image)
} else if !reflect.DeepEqual(
actualEnvVars,
expectedEnvVars) {
t.Fatalf("wrong env vars %v", conf.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Env)
t.Fatalf("wrong env vars %v", template.Spec.DeprecatedContainer.Env)
}
}
@ -131,14 +131,14 @@ func TestServiceCreateWithRequests(t *testing.T) {
corev1.ResourceMemory: parseQuantity(t, "64Mi"),
}
conf, err := servinglib.GetConfiguration(created)
template, err := servinglib.GetRevisionTemplate(created)
if err != nil {
t.Fatal(err)
} else if !reflect.DeepEqual(
conf.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Resources.Requests,
template.Spec.DeprecatedContainer.Resources.Requests,
expectedRequestsVars) {
t.Fatalf("wrong requests vars %v", conf.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Resources.Requests)
t.Fatalf("wrong requests vars %v", template.Spec.DeprecatedContainer.Resources.Requests)
}
}
@ -157,14 +157,14 @@ func TestServiceCreateWithLimits(t *testing.T) {
corev1.ResourceMemory: parseQuantity(t, "1024Mi"),
}
conf, err := servinglib.GetConfiguration(created)
template, err := servinglib.GetRevisionTemplate(created)
if err != nil {
t.Fatal(err)
} else if !reflect.DeepEqual(
conf.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Resources.Limits,
template.Spec.DeprecatedContainer.Resources.Limits,
expectedLimitsVars) {
t.Fatalf("wrong limits vars %v", conf.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Resources.Limits)
t.Fatalf("wrong limits vars %v", template.Spec.DeprecatedContainer.Resources.Limits)
}
}
@ -186,21 +186,21 @@ func TestServiceCreateRequestsLimitsCPU(t *testing.T) {
corev1.ResourceCPU: parseQuantity(t, "1000m"),
}
conf, err := servinglib.GetConfiguration(created)
template, err := servinglib.GetRevisionTemplate(created)
if err != nil {
t.Fatal(err)
} else {
if !reflect.DeepEqual(
conf.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Resources.Requests,
template.Spec.DeprecatedContainer.Resources.Requests,
expectedRequestsVars) {
t.Fatalf("wrong requests vars %v", conf.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Resources.Requests)
t.Fatalf("wrong requests vars %v", template.Spec.DeprecatedContainer.Resources.Requests)
}
if !reflect.DeepEqual(
conf.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Resources.Limits,
template.Spec.DeprecatedContainer.Resources.Limits,
expectedLimitsVars) {
t.Fatalf("wrong limits vars %v", conf.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Resources.Limits)
t.Fatalf("wrong limits vars %v", template.Spec.DeprecatedContainer.Resources.Limits)
}
}
}
@ -223,21 +223,21 @@ func TestServiceCreateRequestsLimitsMemory(t *testing.T) {
corev1.ResourceMemory: parseQuantity(t, "1024Mi"),
}
conf, err := servinglib.GetConfiguration(created)
template, err := servinglib.GetRevisionTemplate(created)
if err != nil {
t.Fatal(err)
} else {
if !reflect.DeepEqual(
conf.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Resources.Requests,
template.Spec.DeprecatedContainer.Resources.Requests,
expectedRequestsVars) {
t.Fatalf("wrong requests vars %v", conf.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Resources.Requests)
t.Fatalf("wrong requests vars %v", template.Spec.DeprecatedContainer.Resources.Requests)
}
if !reflect.DeepEqual(
conf.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Resources.Limits,
template.Spec.DeprecatedContainer.Resources.Limits,
expectedLimitsVars) {
t.Fatalf("wrong limits vars %v", conf.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Resources.Limits)
t.Fatalf("wrong limits vars %v", template.Spec.DeprecatedContainer.Resources.Limits)
}
}
}
@ -264,21 +264,21 @@ func TestServiceCreateRequestsLimitsCPUMemory(t *testing.T) {
corev1.ResourceMemory: parseQuantity(t, "1024Mi"),
}
conf, err := servinglib.GetConfiguration(created)
template, err := servinglib.GetRevisionTemplate(created)
if err != nil {
t.Fatal(err)
} else {
if !reflect.DeepEqual(
conf.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Resources.Requests,
template.Spec.DeprecatedContainer.Resources.Requests,
expectedRequestsVars) {
t.Fatalf("wrong requests vars %v", conf.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Resources.Requests)
t.Fatalf("wrong requests vars %v", template.Spec.DeprecatedContainer.Resources.Requests)
}
if !reflect.DeepEqual(
conf.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Resources.Limits,
template.Spec.DeprecatedContainer.Resources.Limits,
expectedLimitsVars) {
t.Fatalf("wrong limits vars %v", conf.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Resources.Limits)
t.Fatalf("wrong limits vars %v", template.Spec.DeprecatedContainer.Resources.Limits)
}
}
}
@ -304,11 +304,11 @@ func TestServiceCreateImageForce(t *testing.T) {
} else if !action.Matches("create", "services") {
t.Fatalf("Bad action %v", action)
}
conf, err := servinglib.GetConfiguration(created)
template, err := servinglib.GetRevisionTemplate(created)
if err != nil {
t.Fatal(err)
} else if conf.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Image != "gcr.io/foo/bar:v2" {
t.Fatalf("wrong image set: %v", conf.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Image)
} else if template.Spec.DeprecatedContainer.Image != "gcr.io/foo/bar:v2" {
t.Fatalf("wrong image set: %v", template.Spec.DeprecatedContainer.Image)
} else if !strings.Contains(output, "foo") || !strings.Contains(output, "default") {
t.Fatalf("wrong output: %s", output)
}
@ -333,19 +333,19 @@ func TestServiceCreateEnvForce(t *testing.T) {
"A": "CATS",
"B": "LIONS"}
conf, err := servinglib.GetConfiguration(created)
actualEnvVars, err := servinglib.EnvToMap(conf.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Env)
template, err := servinglib.GetRevisionTemplate(created)
actualEnvVars, err := servinglib.EnvToMap(template.Spec.DeprecatedContainer.Env)
if err != nil {
t.Fatal(err)
}
if err != nil {
t.Fatal(err)
} else if conf.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Image != "gcr.io/foo/bar:v2" {
t.Fatalf("wrong image set: %v", conf.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Image)
} else if template.Spec.DeprecatedContainer.Image != "gcr.io/foo/bar:v2" {
t.Fatalf("wrong image set: %v", template.Spec.DeprecatedContainer.Image)
} else if !reflect.DeepEqual(
actualEnvVars,
expectedEnvVars) {
t.Fatalf("wrong env vars:%v", conf.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Env)
t.Fatalf("wrong env vars:%v", template.Spec.DeprecatedContainer.Env)
} else if !strings.Contains(output, "foo") || !strings.Contains(output, "default") {
t.Fatalf("wrong output: %s", output)
}

8
pkg/kn/commands/service_update.go
View File

@ -17,7 +17,6 @@ package commands
import (
"errors"
serving_lib "github.com/knative/client/pkg/serving"
"github.com/spf13/cobra"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)
@ -53,12 +52,9 @@ func NewServiceUpdateCommand(p *KnParams) *cobra.Command {
if err != nil {
return err
}
service = service.DeepCopy()
config, err := serving_lib.GetConfiguration(service)
if err != nil {
return err
}
err = editFlags.Apply(config, cmd)
err = editFlags.Apply(service, cmd)
if err != nil {
return err
}

59
pkg/kn/commands/service_update_test.go
View File

@ -93,12 +93,12 @@ func TestServiceUpdateImage(t *testing.T) {
},
}
config, err := servinglib.GetConfiguration(orig)
template, err := servinglib.GetRevisionTemplate(orig)
if err != nil {
t.Fatal(err)
}
servinglib.UpdateImage(config, "gcr.io/foo/bar:baz")
servinglib.UpdateImage(template, "gcr.io/foo/bar:baz")
action, updated, _, err := fakeServiceUpdate(orig, []string{
"service", "update", "foo", "--image", "gcr.io/foo/quux:xyzzy"})
@ -108,11 +108,12 @@ func TestServiceUpdateImage(t *testing.T) {
} else if !action.Matches("update", "services") {
t.Fatalf("Bad action %v", action)
}
conf, err := servinglib.GetConfiguration(updated)
template, err = servinglib.GetRevisionTemplate(updated)
if err != nil {
t.Fatal(err)
} else if conf.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Image != "gcr.io/foo/quux:xyzzy" {
t.Fatalf("wrong image set: %v", conf.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Image)
} else if template.Spec.DeprecatedContainer.Image != "gcr.io/foo/quux:xyzzy" {
t.Fatalf("wrong image set: %v", template.Spec.DeprecatedContainer.Image)
}
}
@ -139,12 +140,12 @@ func TestServiceUpdateEnv(t *testing.T) {
},
}
config, err := servinglib.GetConfiguration(orig)
template, err := servinglib.GetRevisionTemplate(orig)
if err != nil {
t.Fatal(err)
}
servinglib.UpdateImage(config, "gcr.io/foo/bar:baz")
servinglib.UpdateImage(template, "gcr.io/foo/bar:baz")
action, updated, _, err := fakeServiceUpdate(orig, []string{
"service", "update", "foo", "-e", "TARGET=Awesome"})
@ -159,13 +160,13 @@ func TestServiceUpdateEnv(t *testing.T) {
Value: "Awesome",
}
conf, err := servinglib.GetConfiguration(updated)
template, err = servinglib.GetRevisionTemplate(updated)
if err != nil {
t.Fatal(err)
} else if conf.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Image != "gcr.io/foo/bar:baz" {
t.Fatalf("wrong image set: %v", conf.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Image)
} else if conf.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Env[0] != expectedEnvVar {
t.Fatalf("wrong env set: %v", conf.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Env)
} else if template.Spec.DeprecatedContainer.Image != "gcr.io/foo/bar:baz" {
t.Fatalf("wrong image set: %v", template.Spec.DeprecatedContainer.Image)
} else if template.Spec.DeprecatedContainer.Env[0] != expectedEnvVar {
t.Fatalf("wrong env set: %v", template.Spec.DeprecatedContainer.Env)
}
}
@ -189,20 +190,20 @@ func TestServiceUpdateRequestsLimitsCPU(t *testing.T) {
corev1.ResourceMemory: resource.MustParse("1024Mi"),
}
newConfig, err := servinglib.GetConfiguration(updated)
newTemplate, err := servinglib.GetRevisionTemplate(updated)
if err != nil {
t.Fatal(err)
} else {
if !reflect.DeepEqual(
newConfig.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Resources.Requests,
newTemplate.Spec.DeprecatedContainer.Resources.Requests,
expectedRequestsVars) {
t.Fatalf("wrong requests vars %v", newConfig.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Resources.Requests)
t.Fatalf("wrong requests vars %v", newTemplate.Spec.DeprecatedContainer.Resources.Requests)
}
if !reflect.DeepEqual(
newConfig.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Resources.Limits,
newTemplate.Spec.DeprecatedContainer.Resources.Limits,
expectedLimitsVars) {
t.Fatalf("wrong limits vars %v", newConfig.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Resources.Limits)
t.Fatalf("wrong limits vars %v", newTemplate.Spec.DeprecatedContainer.Resources.Limits)
}
}
}
@ -227,20 +228,20 @@ func TestServiceUpdateRequestsLimitsMemory(t *testing.T) {
corev1.ResourceMemory: resource.MustParse("2048Mi"),
}
newConfig, err := servinglib.GetConfiguration(updated)
newTemplate, err := servinglib.GetRevisionTemplate(updated)
if err != nil {
t.Fatal(err)
} else {
if !reflect.DeepEqual(
newConfig.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Resources.Requests,
newTemplate.Spec.DeprecatedContainer.Resources.Requests,
expectedRequestsVars) {
t.Fatalf("wrong requests vars %v", newConfig.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Resources.Requests)
t.Fatalf("wrong requests vars %v", newTemplate.Spec.DeprecatedContainer.Resources.Requests)
}
if !reflect.DeepEqual(
newConfig.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Resources.Limits,
newTemplate.Spec.DeprecatedContainer.Resources.Limits,
expectedLimitsVars) {
t.Fatalf("wrong limits vars %v", newConfig.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Resources.Limits)
t.Fatalf("wrong limits vars %v", newTemplate.Spec.DeprecatedContainer.Resources.Limits)
}
}
}
@ -267,20 +268,20 @@ func TestServiceUpdateRequestsLimitsCPU_and_Memory(t *testing.T) {
corev1.ResourceMemory: resource.MustParse("2048Mi"),
}
newConfig, err := servinglib.GetConfiguration(updated)
newTemplate, err := servinglib.GetRevisionTemplate(updated)
if err != nil {
t.Fatal(err)
} else {
if !reflect.DeepEqual(
newConfig.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Resources.Requests,
newTemplate.Spec.DeprecatedContainer.Resources.Requests,
expectedRequestsVars) {
t.Fatalf("wrong requests vars %v", newConfig.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Resources.Requests)
t.Fatalf("wrong requests vars %v", newTemplate.Spec.DeprecatedContainer.Resources.Requests)
}
if !reflect.DeepEqual(
newConfig.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Resources.Limits,
newTemplate.Spec.DeprecatedContainer.Resources.Limits,
expectedLimitsVars) {
t.Fatalf("wrong limits vars %v", newConfig.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Resources.Limits)
t.Fatalf("wrong limits vars %v", newTemplate.Spec.DeprecatedContainer.Resources.Limits)
}
}
}
@ -308,12 +309,12 @@ func createMockServiceWithResources(t *testing.T, requestCPU, requestMemory, lim
},
}
config, err := servinglib.GetConfiguration(service)
template, err := servinglib.GetRevisionTemplate(service)
if err != nil {
t.Fatal(err)
}
config.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Resources = corev1.ResourceRequirements{
template.Spec.DeprecatedContainer.Resources = corev1.ResourceRequirements{
Requests: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse(requestCPU),
corev1.ResourceMemory: resource.MustParse(requestMemory),

99
pkg/serving/config_changes.go
View File

@ -24,28 +24,13 @@ import (
// Give the configuration all the env var values listed in the given map of
// vars. Does not touch any environment variables not mentioned, but it can add
// new env vars and change the values of existing ones.
func UpdateEnvVars(config *servingv1alpha1.ConfigurationSpec, vars map[string]string) error {
set := make(map[string]bool)
for i, _ := range config.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Env {
envVar := &config.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Env[i]
value, present := vars[envVar.Name]
if present {
envVar.Value = value
set[envVar.Name] = true
}
}
for name, value := range vars {
if !set[name] {
config.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Env = append(
config.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Env,
corev1.EnvVar{
Name: name,
Value: value,
})
}
func UpdateEnvVars(template *servingv1alpha1.RevisionTemplateSpec, vars map[string]string) error {
container, err := extractContainer(template)
if err != nil {
return err
}
container.Env = updateEnvVarsFromMap(container.Env, vars)
return nil
}
// Utility function to translate between the API list form of env vars, and the
@ -55,34 +40,88 @@ func EnvToMap(vars []corev1.EnvVar) (map[string]string, error) {
for _, envVar := range vars {
_, present := result[envVar.Name]
if present {
return nil, fmt.Errorf("Env var name present more than once: %v", envVar.Name)
return nil, fmt.Errorf("env var name present more than once: %v", envVar.Name)
}
result[envVar.Name] = envVar.Value
}
return result, nil
}
func UpdateImage(config *servingv1alpha1.ConfigurationSpec, image string) error {
config.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Image = image
// Update a given image
func UpdateImage(template *servingv1alpha1.RevisionTemplateSpec, image string) error {
container, err := extractContainer(template)
if err != nil {
return err
}
container.Image = image
return nil
}
func UpdateResources(config *servingv1alpha1.ConfigurationSpec, requestsResourceList corev1.ResourceList, limitsResourceList corev1.ResourceList) error {
if config.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Resources.Requests == nil {
config.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Resources.Requests = corev1.ResourceList{}
func UpdateResources(template *servingv1alpha1.RevisionTemplateSpec, requestsResourceList corev1.ResourceList, limitsResourceList corev1.ResourceList) error {
container, err := extractContainer(template)
if err != nil {
return err
}
if container.Resources.Requests == nil {
container.Resources.Requests = corev1.ResourceList{}
}
for k, v := range requestsResourceList {
config.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Resources.Requests[k] = v
container.Resources.Requests[k] = v
}
if config.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Resources.Limits == nil {
config.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Resources.Limits = corev1.ResourceList{}
if container.Resources.Limits == nil {
container.Resources.Limits = corev1.ResourceList{}
}
for k, v := range limitsResourceList {
config.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Resources.Limits[k] = v
container.Resources.Limits[k] = v
}
return nil
}
// =======================================================================================
func usesOldV1alpha1ContainerField(revision *servingv1alpha1.RevisionTemplateSpec) bool {
return revision.Spec.DeprecatedContainer != nil
}
func extractContainer(template *servingv1alpha1.RevisionTemplateSpec) (*corev1.Container, error) {
if usesOldV1alpha1ContainerField(template) {
return template.Spec.DeprecatedContainer, nil
}
containers := template.Spec.Containers
if len(containers) == 0 {
return nil, fmt.Errorf("internal: no container set in spec.template.spec.containers")
}
if len(containers) > 1 {
return nil, fmt.Errorf("internal: can't extract container for updating environment"+
" variables as the configuration contains "+
"more than one container (i.e. %d containers)", len(containers))
}
return &containers[0], nil
}
func updateEnvVarsFromMap(env []corev1.EnvVar, vars map[string]string) []corev1.EnvVar {
set := make(map[string]bool)
for i, _ := range env {
envVar := &env[i]
value, present := vars[envVar.Name]
if present {
envVar.Value = value
set[envVar.Name] = true
}
}
for name, value := range vars {
if !set[name] {
env = append(
env,
corev1.EnvVar{
Name: name,
Value: value,
})
}
}
return env
}

114
pkg/serving/config_changes_test.go
View File

@ -15,6 +15,7 @@
package serving
import (
"github.com/knative/serving/pkg/apis/serving/v1beta1"
"reflect"
"testing"
@ -23,16 +24,25 @@ import (
)
func TestUpdateEnvVarsNew(t *testing.T) {
config := getEmptyConfigurationSpec()
template, container := getV1alpha1RevisionTemplateWithOldFields()
testUpdateEnvVarsNew(t, template, container)
assertNoV1alpha1(t, template)
template, container = getV1alpha1Config()
testUpdateEnvVarsNew(t, template, container)
assertNoV1alpha1Old(t, template)
}
func testUpdateEnvVarsNew(t *testing.T, template *servingv1alpha1.RevisionTemplateSpec, container *corev1.Container) {
env := map[string]string{
"a": "foo",
"b": "bar",
}
err := UpdateEnvVars(&config, env)
err := UpdateEnvVars(template, env)
if err != nil {
t.Fatal(err)
}
found, err := EnvToMap(config.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Env)
found, err := EnvToMap(container.Env)
if err != nil {
t.Fatal(err)
}
@ -41,14 +51,24 @@ func TestUpdateEnvVarsNew(t *testing.T) {
}
}
func TestUpdateEnvVarsAppend(t *testing.T) {
config := getEmptyConfigurationSpec()
config.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Env = []corev1.EnvVar{
corev1.EnvVar{Name: "a", Value: "foo"}}
func TestUpdateEnvVarsAppendOld(t *testing.T) {
template, container := getV1alpha1RevisionTemplateWithOldFields()
testUpdateEnvVarsAppendOld(t, template, container)
assertNoV1alpha1(t, template)
template, container = getV1alpha1Config()
testUpdateEnvVarsAppendOld(t, template, container)
assertNoV1alpha1Old(t, template)
}
func testUpdateEnvVarsAppendOld(t *testing.T, template *servingv1alpha1.RevisionTemplateSpec, container *corev1.Container) {
container.Env = []corev1.EnvVar{
{Name: "a", Value: "foo"},
}
env := map[string]string{
"b": "bar",
}
err := UpdateEnvVars(&config, env)
err := UpdateEnvVars(template, env)
if err != nil {
t.Fatal(err)
}
@ -58,23 +78,32 @@ func TestUpdateEnvVarsAppend(t *testing.T) {
"b": "bar",
}
found, err := EnvToMap(config.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Env)
found, err := EnvToMap(container.Env)
if err != nil {
t.Fatal(err)
}
if !reflect.DeepEqual(expected, found) {
t.Fatalf("Env did not match expected %v found %v", env, found)
t.Fatalf("Env did not match expected %v, found %v", env, found)
}
}
func TestUpdateEnvVarsModify(t *testing.T) {
config := getEmptyConfigurationSpec()
config.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Env = []corev1.EnvVar{
template, container := getV1alpha1RevisionTemplateWithOldFields()
testUpdateEnvVarsModify(t, template, container)
assertNoV1alpha1(t, template)
template, container = getV1alpha1Config()
testUpdateEnvVarsModify(t, template, container)
assertNoV1alpha1Old(t, template)
}
func testUpdateEnvVarsModify(t *testing.T, revision *servingv1alpha1.RevisionTemplateSpec, container *corev1.Container) {
container.Env = []corev1.EnvVar{
corev1.EnvVar{Name: "a", Value: "foo"}}
env := map[string]string{
"a": "fancy",
}
err := UpdateEnvVars(&config, env)
err := UpdateEnvVars(revision, env)
if err != nil {
t.Fatal(err)
}
@ -83,25 +112,34 @@ func TestUpdateEnvVarsModify(t *testing.T) {
"a": "fancy",
}
found, err := EnvToMap(config.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Env)
found, err := EnvToMap(container.Env)
if err != nil {
t.Fatal(err)
}
if !reflect.DeepEqual(expected, found) {
t.Fatalf("Env did not match expected %v found %v", env, found)
t.Fatalf("Env did not match expected %v, found %v", env, found)
}
}
func TestUpdateEnvVarsBoth(t *testing.T) {
config := getEmptyConfigurationSpec()
config.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Env = []corev1.EnvVar{
template, container := getV1alpha1RevisionTemplateWithOldFields()
testUpdateEnvVarsBoth(t, template, container)
assertNoV1alpha1(t, template)
template, container = getV1alpha1Config()
testUpdateEnvVarsBoth(t, template, container)
assertNoV1alpha1Old(t, template)
}
func testUpdateEnvVarsBoth(t *testing.T, template *servingv1alpha1.RevisionTemplateSpec, container *corev1.Container) {
container.Env = []corev1.EnvVar{
corev1.EnvVar{Name: "a", Value: "foo"},
corev1.EnvVar{Name: "c", Value: "caroline"}}
env := map[string]string{
"a": "fancy",
"b": "boo",
}
err := UpdateEnvVars(&config, env)
err := UpdateEnvVars(template, env)
if err != nil {
t.Fatal(err)
}
@ -112,21 +150,49 @@ func TestUpdateEnvVarsBoth(t *testing.T) {
"c": "caroline",
}
found, err := EnvToMap(config.DeprecatedRevisionTemplate.Spec.DeprecatedContainer.Env)
found, err := EnvToMap(container.Env)
if err != nil {
t.Fatal(err)
}
if !reflect.DeepEqual(expected, found) {
t.Fatalf("Env did not match expected %v found %v", env, found)
t.Fatalf("Env did not match expected %v, found %v", env, found)
}
}
func getEmptyConfigurationSpec() servingv1alpha1.ConfigurationSpec {
return servingv1alpha1.ConfigurationSpec{
DeprecatedRevisionTemplate: &servingv1alpha1.RevisionTemplateSpec{
// =========================================================================================================
func getV1alpha1RevisionTemplateWithOldFields() (*servingv1alpha1.RevisionTemplateSpec, *corev1.Container) {
container := &corev1.Container{}
template := &servingv1alpha1.RevisionTemplateSpec{
Spec: servingv1alpha1.RevisionSpec{
DeprecatedContainer: &corev1.Container{},
DeprecatedContainer: container,
},
}
return template, container
}
func getV1alpha1Config() (*servingv1alpha1.RevisionTemplateSpec, *corev1.Container) {
containers := []corev1.Container{{}}
template := &servingv1alpha1.RevisionTemplateSpec{
Spec: servingv1alpha1.RevisionSpec{
RevisionSpec: v1beta1.RevisionSpec{
PodSpec: v1beta1.PodSpec{
Containers: containers,
},
},
},
}
return template, &containers[0]
}
func assertNoV1alpha1Old(t *testing.T, template *servingv1alpha1.RevisionTemplateSpec) {
if template.Spec.DeprecatedContainer != nil {
t.Error("Assuming only new v1alpha1 fields but found spec.container")
}
}
func assertNoV1alpha1(t *testing.T, template *servingv1alpha1.RevisionTemplateSpec) {
if template.Spec.Containers != nil {
t.Error("Assuming only old v1alpha1 fields but found spec.template")
}
}

22
pkg/serving/service.go
View File

@ -20,7 +20,25 @@ import (
servingv1alpha1 "github.com/knative/serving/pkg/apis/serving/v1alpha1"
)
func GetConfiguration(service *servingv1alpha1.Service) (*servingv1alpha1.ConfigurationSpec, error) {
// Get the revision template associated with a service.
// Depending on the structure returned either the new v1beta1 fields or the
// 'old' v1alpha1 fields are looked up.
// The returned revision template can be updated in place.
// An error is returned if no revision template could be extracted
func GetRevisionTemplate(service *servingv1alpha1.Service) (*servingv1alpha1.RevisionTemplateSpec, error) {
// Try v1beta1 field first
if service.Spec.Template != nil {
return service.Spec.Template, nil
}
config, err := getConfiguration(service)
if err != nil {
return nil, err
}
return config.DeprecatedRevisionTemplate, nil
}
func getConfiguration(service *servingv1alpha1.Service) (*servingv1alpha1.ConfigurationSpec, error) {
if service.Spec.DeprecatedRunLatest != nil {
return &service.Spec.DeprecatedRunLatest.Configuration, nil
} else if service.Spec.DeprecatedRelease != nil {
@ -28,6 +46,6 @@ func GetConfiguration(service *servingv1alpha1.Service) (*servingv1alpha1.Config
} else if service.Spec.DeprecatedPinned != nil {
return &service.Spec.DeprecatedPinned.Configuration, nil
} else {
return nil, errors.New("Service does not specify a Configuration")
return nil, errors.New("service does not specify a Configuration")
}
}