Skip to content

Commit

Permalink
Rename accessors to be less like methods that get from the server (#275)
Browse files Browse the repository at this point in the history
  • Loading branch information
@sixolet @knative-prow-robot
sixolet authored and knative-prow-robot committed Jul 19, 2019
1 parent 0b348d7 commit 8d7d227
Show file tree
Hide file tree
Showing 6 changed files with 74 additions and 46 deletions.
2 changes: 1 addition & 1 deletion pkg/kn/commands/service/configuration_edit_flags.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func (p *ConfigurationEditFlags) AddCreateFlags(command *cobra.Command) {

func (p *ConfigurationEditFlags) Apply(service *servingv1alpha1.Service, cmd *cobra.Command) error {

template, err := servinglib.GetRevisionTemplate(service)
template, err := servinglib.RevisionTemplateOfService(service)
if err != nil {
return err
}
Expand Down
22 changes: 11 additions & 11 deletions pkg/kn/commands/service/service_create_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ func TestServiceCreateImage(t *testing.T) {
} else if !action.Matches("create", "services") {
t.Fatalf("Bad action %v", action)
}
template, err := servinglib.GetRevisionTemplate(created)
template, err := servinglib.RevisionTemplateOfService(created)
if err != nil {
t.Fatal(err)
} else if template.Spec.DeprecatedContainer.Image != "gcr.io/foo/bar:baz" {
Expand All @@ -140,7 +140,7 @@ func TestServiceCreateImageSync(t *testing.T) {
} else if !action.Matches("create", "services") {
t.Fatalf("Bad action %v", action)
}
template, err := servinglib.GetRevisionTemplate(created)
template, err := servinglib.RevisionTemplateOfService(created)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -170,7 +170,7 @@ func TestServiceCreateEnv(t *testing.T) {
"A": "DOGS",
"B": "WOLVES"}

template, err := servinglib.GetRevisionTemplate(created)
template, err := servinglib.RevisionTemplateOfService(created)
actualEnvVars, err := servinglib.EnvToMap(template.Spec.DeprecatedContainer.Env)
if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -202,7 +202,7 @@ func TestServiceCreateWithRequests(t *testing.T) {
corev1.ResourceMemory: parseQuantity(t, "64Mi"),
}

template, err := servinglib.GetRevisionTemplate(created)
template, err := servinglib.RevisionTemplateOfService(created)

if err != nil {
t.Fatal(err)
Expand All @@ -228,7 +228,7 @@ func TestServiceCreateWithLimits(t *testing.T) {
corev1.ResourceMemory: parseQuantity(t, "1024Mi"),
}

template, err := servinglib.GetRevisionTemplate(created)
template, err := servinglib.RevisionTemplateOfService(created)

if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -257,7 +257,7 @@ func TestServiceCreateRequestsLimitsCPU(t *testing.T) {
corev1.ResourceCPU: parseQuantity(t, "1000m"),
}

template, err := servinglib.GetRevisionTemplate(created)
template, err := servinglib.RevisionTemplateOfService(created)

if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -294,7 +294,7 @@ func TestServiceCreateRequestsLimitsMemory(t *testing.T) {
corev1.ResourceMemory: parseQuantity(t, "1024Mi"),
}

template, err := servinglib.GetRevisionTemplate(created)
template, err := servinglib.RevisionTemplateOfService(created)

if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -324,7 +324,7 @@ func TestServiceCreateMaxMinScale(t *testing.T) {
t.Fatalf("Bad action %v", action)
}

template, err := servinglib.GetRevisionTemplate(created)
template, err := servinglib.RevisionTemplateOfService(created)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -371,7 +371,7 @@ func TestServiceCreateRequestsLimitsCPUMemory(t *testing.T) {
corev1.ResourceMemory: parseQuantity(t, "1024Mi"),
}

template, err := servinglib.GetRevisionTemplate(created)
template, err := servinglib.RevisionTemplateOfService(created)

if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -421,7 +421,7 @@ func TestServiceCreateImageForce(t *testing.T) {
} else if !action.Matches("update", "services") {
t.Fatalf("Bad action %v", action)
}
template, err := servinglib.GetRevisionTemplate(created)
template, err := servinglib.RevisionTemplateOfService(created)
if err != nil {
t.Fatal(err)
} else if template.Spec.DeprecatedContainer.Image != "gcr.io/foo/bar:v2" {
Expand Down Expand Up @@ -450,7 +450,7 @@ func TestServiceCreateEnvForce(t *testing.T) {
"A": "CATS",
"B": "LIONS"}

template, err := servinglib.GetRevisionTemplate(created)
template, err := servinglib.RevisionTemplateOfService(created)
actualEnvVars, err := servinglib.EnvToMap(template.Spec.DeprecatedContainer.Env)
if err != nil {
t.Fatal(err)
Expand Down
18 changes: 9 additions & 9 deletions pkg/kn/commands/service/service_update_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func fakeServiceUpdate(original *v1alpha1.Service, args []string) (
func TestServiceUpdateImage(t *testing.T) {
orig := newEmptyService()

template, err := servinglib.GetRevisionTemplate(orig)
template, err := servinglib.RevisionTemplateOfService(orig)
if err != nil {
t.Fatal(err)
}
Expand All @@ -83,7 +83,7 @@ func TestServiceUpdateImage(t *testing.T) {
t.Fatalf("Bad action %v", action)
}

template, err = servinglib.GetRevisionTemplate(updated)
template, err = servinglib.RevisionTemplateOfService(updated)
if err != nil {
t.Fatal(err)
} else if template.Spec.DeprecatedContainer.Image != "gcr.io/foo/quux:xyzzy" {
Expand Down Expand Up @@ -112,7 +112,7 @@ func TestServiceUpdateMaxMinScale(t *testing.T) {
t.Fatalf("Bad action %v", action)
}

template, err := servinglib.GetRevisionTemplate(updated)
template, err := servinglib.RevisionTemplateOfService(updated)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -161,7 +161,7 @@ func TestServiceUpdateEnv(t *testing.T) {
},
}

template, err := servinglib.GetRevisionTemplate(orig)
template, err := servinglib.RevisionTemplateOfService(orig)
if err != nil {
t.Fatal(err)
}
Expand All @@ -181,7 +181,7 @@ func TestServiceUpdateEnv(t *testing.T) {
Value: "Awesome",
}

template, err = servinglib.GetRevisionTemplate(updated)
template, err = servinglib.RevisionTemplateOfService(updated)
if err != nil {
t.Fatal(err)
} else if template.Spec.DeprecatedContainer.Image != "gcr.io/foo/bar:baz" {
Expand Down Expand Up @@ -211,7 +211,7 @@ func TestServiceUpdateRequestsLimitsCPU(t *testing.T) {
corev1.ResourceMemory: resource.MustParse("1024Mi"),
}

newTemplate, err := servinglib.GetRevisionTemplate(updated)
newTemplate, err := servinglib.RevisionTemplateOfService(updated)
if err != nil {
t.Fatal(err)
} else {
Expand Down Expand Up @@ -249,7 +249,7 @@ func TestServiceUpdateRequestsLimitsMemory(t *testing.T) {
corev1.ResourceMemory: resource.MustParse("2048Mi"),
}

newTemplate, err := servinglib.GetRevisionTemplate(updated)
newTemplate, err := servinglib.RevisionTemplateOfService(updated)
if err != nil {
t.Fatal(err)
} else {
Expand Down Expand Up @@ -289,7 +289,7 @@ func TestServiceUpdateRequestsLimitsCPU_and_Memory(t *testing.T) {
corev1.ResourceMemory: resource.MustParse("2048Mi"),
}

newTemplate, err := servinglib.GetRevisionTemplate(updated)
newTemplate, err := servinglib.RevisionTemplateOfService(updated)
if err != nil {
t.Fatal(err)
} else {
Expand Down Expand Up @@ -354,7 +354,7 @@ func createMockServiceWithResources(t *testing.T, requestCPU, requestMemory, lim
},
}

template, err := servinglib.GetRevisionTemplate(service)
template, err := servinglib.RevisionTemplateOfService(service)
if err != nil {
t.Fatal(err)
}
Expand Down
28 changes: 4 additions & 24 deletions pkg/serving/config_changes.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import (
// 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(template *servingv1alpha1.RevisionTemplateSpec, vars map[string]string) error {
container, err := extractContainer(template)
container, err := ContainerOfRevisionTemplate(template)
if err != nil {
return err
}
Expand Down Expand Up @@ -78,7 +78,7 @@ func EnvToMap(vars []corev1.EnvVar) (map[string]string, error) {

// Update a given image
func UpdateImage(template *servingv1alpha1.RevisionTemplateSpec, image string) error {
container, err := extractContainer(template)
container, err := ContainerOfRevisionTemplate(template)
if err != nil {
return err
}
Expand All @@ -88,7 +88,7 @@ func UpdateImage(template *servingv1alpha1.RevisionTemplateSpec, image string) e

// UpdateContainerPort updates container with a give port
func UpdateContainerPort(template *servingv1alpha1.RevisionTemplateSpec, port int32) error {
container, err := extractContainer(template)
container, err := ContainerOfRevisionTemplate(template)
if err != nil {
return err
}
Expand All @@ -99,7 +99,7 @@ func UpdateContainerPort(template *servingv1alpha1.RevisionTemplateSpec, port in
}

func UpdateResources(template *servingv1alpha1.RevisionTemplateSpec, requestsResourceList corev1.ResourceList, limitsResourceList corev1.ResourceList) error {
container, err := extractContainer(template)
container, err := ContainerOfRevisionTemplate(template)
if err != nil {
return err
}
Expand All @@ -124,26 +124,6 @@ func UpdateResources(template *servingv1alpha1.RevisionTemplateSpec, requestsRes

// =======================================================================================

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 {
Expand Down
48 changes: 48 additions & 0 deletions pkg/serving/revision_template.go
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// Copyright © 2019 The Knative Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package serving

import (
"fmt"

servingv1alpha1 "github.com/knative/serving/pkg/apis/serving/v1alpha1"
corev1 "k8s.io/api/core/v1"
)

func ContainerOfRevisionTemplate(template *servingv1alpha1.RevisionTemplateSpec) (*corev1.Container, error) {
return ContainerOfRevisionSpec(&template.Spec)
}

func ContainerOfRevisionSpec(revisionSpec *servingv1alpha1.RevisionSpec) (*corev1.Container, error) {
if usesOldV1alpha1ContainerField(revisionSpec) {
return revisionSpec.DeprecatedContainer, nil
}
containers := revisionSpec.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 usesOldV1alpha1ContainerField(revisionSpec *servingv1alpha1.RevisionSpec) bool {
return revisionSpec.DeprecatedContainer != nil
}
2 changes: 1 addition & 1 deletion pkg/serving/service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (
// '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) {
func RevisionTemplateOfService(service *servingv1alpha1.Service) (*servingv1alpha1.RevisionTemplateSpec, error) {
// Try v1beta1 field first
if service.Spec.Template != nil {
return service.Spec.Template, nil
Expand Down

0 comments on commit 8d7d227

Please sign in to comment.