diff --git a/configuration.yaml b/configuration.yaml index 108686b76ad3..1ae312d75515 100644 --- a/configuration.yaml +++ b/configuration.yaml @@ -23,3 +23,5 @@ spec: kind: Configuration plural: configurations scope: Namespaced + subresources: + status: {} diff --git a/pkg/apis/ela/v1alpha1/configuration_types.go b/pkg/apis/ela/v1alpha1/configuration_types.go index 9d88d70b437c..28555c3103cc 100644 --- a/pkg/apis/ela/v1alpha1/configuration_types.go +++ b/pkg/apis/ela/v1alpha1/configuration_types.go @@ -47,12 +47,6 @@ type Configuration struct { // ConfigurationSpec holds the desired state of the Configuration (from the client). type ConfigurationSpec struct { - // TODO: Generation does not work correctly with CRD. They are scrubbed - // by the APIserver (https://github.com/kubernetes/kubernetes/issues/58778) - // So, we add Generation here. Once that gets fixed, remove this and use - // ObjectMeta.Generation instead. - Generation int64 `json:"generation,omitempty"` - // Build optionally holds the specification for the build to // perform to produce the Revision's container image. Build *build.BuildSpec `json:"build,omitempty"` @@ -122,14 +116,6 @@ type ConfigurationList struct { Items []Configuration `json:"items"` } -func (r *Configuration) GetGeneration() int64 { - return r.Spec.Generation -} - -func (r *Configuration) SetGeneration(generation int64) { - r.Spec.Generation = generation -} - func (r *Configuration) GetSpecJSON() ([]byte, error) { return json.Marshal(r.Spec) } diff --git a/pkg/apis/ela/v1alpha1/configuration_types_test.go b/pkg/apis/ela/v1alpha1/configuration_types_test.go index 4d97a1cdb7d3..fcf4befe04a4 100644 --- a/pkg/apis/ela/v1alpha1/configuration_types_test.go +++ b/pkg/apis/ela/v1alpha1/configuration_types_test.go @@ -18,18 +18,6 @@ import ( corev1 "k8s.io/api/core/v1" ) -func TestConfigurationGeneration(t *testing.T) { - config := Configuration{} - if e, a := int64(0), config.GetGeneration(); e != a { - t.Errorf("empty revision generation should be 0 was: %d", a) - } - - config.SetGeneration(5) - if e, a := int64(5), config.GetGeneration(); e != a { - t.Errorf("getgeneration mismatch expected: %d got: %d", e, a) - } -} - func TestConfigurationIsReady(t *testing.T) { cases := []struct { name string diff --git a/pkg/apis/ela/v1alpha1/revision_types.go b/pkg/apis/ela/v1alpha1/revision_types.go index 750cdfa8eb7b..32700fd7af2e 100644 --- a/pkg/apis/ela/v1alpha1/revision_types.go +++ b/pkg/apis/ela/v1alpha1/revision_types.go @@ -70,12 +70,6 @@ const ( // RevisionSpec holds the desired state of the Revision (from the client). type RevisionSpec struct { - // TODO: Generation does not work correctly with CRD. They are scrubbed - // by the APIserver (https://github.com/kubernetes/kubernetes/issues/58778) - // So, we add Generation here. Once that gets fixed, remove this and use - // ObjectMeta.Generation instead. - Generation int64 `json:"generation,omitempty"` - // ServingState holds a value describing the desired state the Kubernetes // resources should be in for this Revision. // Users must not specify this when creating a revision. It is expected @@ -167,14 +161,6 @@ type RevisionList struct { Items []Revision `json:"items"` } -func (r *Revision) GetGeneration() int64 { - return r.Spec.Generation -} - -func (r *Revision) SetGeneration(generation int64) { - r.Spec.Generation = generation -} - func (r *Revision) GetSpecJSON() ([]byte, error) { return json.Marshal(r.Spec) } diff --git a/pkg/apis/ela/v1alpha1/revision_types_test.go b/pkg/apis/ela/v1alpha1/revision_types_test.go index 7620d0dcf5d7..778f64e7dec8 100644 --- a/pkg/apis/ela/v1alpha1/revision_types_test.go +++ b/pkg/apis/ela/v1alpha1/revision_types_test.go @@ -19,19 +19,6 @@ import ( corev1 "k8s.io/api/core/v1" ) -func TestGeneration(t *testing.T) { - r := Revision{} - if a := r.GetGeneration(); a != 0 { - t.Errorf("empty revision generation should be 0 was: %d", a) - } - - r.SetGeneration(5) - if e, a := int64(5), r.GetGeneration(); e != a { - t.Errorf("getgeneration mismatch expected: %d got: %d", e, a) - } - -} - func TestIsReady(t *testing.T) { cases := []struct { name string diff --git a/pkg/apis/ela/v1alpha1/route_types.go b/pkg/apis/ela/v1alpha1/route_types.go index 9494d848e838..df3e7dfc0d61 100644 --- a/pkg/apis/ela/v1alpha1/route_types.go +++ b/pkg/apis/ela/v1alpha1/route_types.go @@ -71,12 +71,6 @@ type TrafficTarget struct { // RouteSpec holds the desired state of the Route (from the client). type RouteSpec struct { - // TODO: Generation does not work correctly with CRD. They are scrubbed - // by the APIserver (https://github.com/kubernetes/kubernetes/issues/58778) - // So, we add Generation here. Once that gets fixed, remove this and use - // ObjectMeta.Generation instead. - Generation int64 `json:"generation,omitempty"` - // Traffic specifies how to distribute traffic over a collection of Elafros Revisions and Configurations. Traffic []TrafficTarget `json:"traffic,omitempty"` } @@ -141,14 +135,6 @@ type RouteList struct { Items []Route `json:"items"` } -func (r *Route) GetGeneration() int64 { - return r.Spec.Generation -} - -func (r *Route) SetGeneration(generation int64) { - r.Spec.Generation = generation -} - func (r *Route) GetSpecJSON() ([]byte, error) { return json.Marshal(r.Spec) } diff --git a/pkg/apis/ela/v1alpha1/service_types.go b/pkg/apis/ela/v1alpha1/service_types.go index 7bf24a86f3ed..053c1d59e8fc 100644 --- a/pkg/apis/ela/v1alpha1/service_types.go +++ b/pkg/apis/ela/v1alpha1/service_types.go @@ -36,14 +36,8 @@ type Service struct { } // ServiceSpec represents the configuration for the Service object. -// Exactly one of its members (other than Generation) must be specified. +// Exactly one of its members must be specified. type ServiceSpec struct { - // TODO: Generation does not work correctly with CRD. They are scrubbed - // by the APIserver (https://github.com/kubernetes/kubernetes/issues/58778) - // So, we add Generation here. Once that gets fixed, remove this and use - // ObjectMeta.Generation instead. - Generation int64 `json:"generation,omitempty"` - // RunLatest defines a simple Service. It will automatically // configure a route that keeps the latest ready revision // from the supplied configuration running. @@ -108,14 +102,6 @@ type ServiceList struct { Items []Service `json:"items"` } -func (s *Service) GetGeneration() int64 { - return s.Spec.Generation -} - -func (s *Service) SetGeneration(generation int64) { - s.Spec.Generation = generation -} - func (s *Service) GetSpecJSON() ([]byte, error) { return json.Marshal(s.Spec) } diff --git a/pkg/apis/ela/v1alpha1/service_types_test.go b/pkg/apis/ela/v1alpha1/service_types_test.go index d712aeec09c7..f2960d52bd73 100644 --- a/pkg/apis/ela/v1alpha1/service_types_test.go +++ b/pkg/apis/ela/v1alpha1/service_types_test.go @@ -13,173 +13,160 @@ limitations under the License. package v1alpha1 import ( - "testing" + "testing" - corev1 "k8s.io/api/core/v1" + corev1 "k8s.io/api/core/v1" ) -func TestServiceGeneration(t *testing.T) { - service := Service{} - if got, want := service.GetGeneration(), int64(0); got != want { - t.Errorf("Empty Service generation should be %d, was %d", want, got) - } - - answer := int64(42) - service.SetGeneration(answer) - if got := service.GetGeneration(); got != answer { - t.Errorf("GetGeneration mismatch; got %d, want %d", got, answer) - } -} - func TestServiceIsReady(t *testing.T) { - cases := []struct { - name string - status ServiceStatus - isReady bool - }{ - { - name: "empty status should not be ready", - status: ServiceStatus{}, - isReady: false, - }, - { - name: "Different condition type should not be ready", - status: ServiceStatus{ - Conditions: []ServiceCondition{ - { - Type: "Foo", - Status: corev1.ConditionTrue, - }, - }, - }, - isReady: false, - }, - { - name: "False condition status should not be ready", - status: ServiceStatus{ - Conditions: []ServiceCondition{ - { - Type: ServiceConditionReady, - Status: corev1.ConditionFalse, - }, - }, - }, - isReady: false, - }, - { - name: "Unknown condition status should not be ready", - status: ServiceStatus{ - Conditions: []ServiceCondition{ - { - Type: ServiceConditionReady, - Status: corev1.ConditionUnknown, - }, - }, - }, - isReady: false, - }, - { - name: "Missing condition status should not be ready", - status: ServiceStatus{ - Conditions: []ServiceCondition{ - { - Type: ServiceConditionReady, - }, - }, - }, - isReady: false, - }, - { - name: "True condition status should be ready", - status: ServiceStatus{ - Conditions: []ServiceCondition{ - { - Type: ServiceConditionReady, - Status: corev1.ConditionTrue, - }, - }, - }, - isReady: true, - }, - { - name: "Multiple conditions with ready status should be ready", - status: ServiceStatus{ - Conditions: []ServiceCondition{ - { - Type: "Foo", - Status: corev1.ConditionTrue, - }, - { - Type: ServiceConditionReady, - Status: corev1.ConditionTrue, - }, - }, - }, - isReady: true, - }, - { - name: "Multiple conditions with ready status false should not be ready", - status: ServiceStatus{ - Conditions: []ServiceCondition{ - { - Type: "Foo", - Status: corev1.ConditionTrue, - }, - { - Type: ServiceConditionReady, - Status: corev1.ConditionFalse, - }, - }, - }, - isReady: false, - }, - } + cases := []struct { + name string + status ServiceStatus + isReady bool + }{ + { + name: "empty status should not be ready", + status: ServiceStatus{}, + isReady: false, + }, + { + name: "Different condition type should not be ready", + status: ServiceStatus{ + Conditions: []ServiceCondition{ + { + Type: "Foo", + Status: corev1.ConditionTrue, + }, + }, + }, + isReady: false, + }, + { + name: "False condition status should not be ready", + status: ServiceStatus{ + Conditions: []ServiceCondition{ + { + Type: ServiceConditionReady, + Status: corev1.ConditionFalse, + }, + }, + }, + isReady: false, + }, + { + name: "Unknown condition status should not be ready", + status: ServiceStatus{ + Conditions: []ServiceCondition{ + { + Type: ServiceConditionReady, + Status: corev1.ConditionUnknown, + }, + }, + }, + isReady: false, + }, + { + name: "Missing condition status should not be ready", + status: ServiceStatus{ + Conditions: []ServiceCondition{ + { + Type: ServiceConditionReady, + }, + }, + }, + isReady: false, + }, + { + name: "True condition status should be ready", + status: ServiceStatus{ + Conditions: []ServiceCondition{ + { + Type: ServiceConditionReady, + Status: corev1.ConditionTrue, + }, + }, + }, + isReady: true, + }, + { + name: "Multiple conditions with ready status should be ready", + status: ServiceStatus{ + Conditions: []ServiceCondition{ + { + Type: "Foo", + Status: corev1.ConditionTrue, + }, + { + Type: ServiceConditionReady, + Status: corev1.ConditionTrue, + }, + }, + }, + isReady: true, + }, + { + name: "Multiple conditions with ready status false should not be ready", + status: ServiceStatus{ + Conditions: []ServiceCondition{ + { + Type: "Foo", + Status: corev1.ConditionTrue, + }, + { + Type: ServiceConditionReady, + Status: corev1.ConditionFalse, + }, + }, + }, + isReady: false, + }, + } - for _, tc := range cases { - if e, a := tc.isReady, tc.status.IsReady(); e != a { - t.Errorf("%q expected: %v got: %v", tc.name, e, a) - } - } + for _, tc := range cases { + if e, a := tc.isReady, tc.status.IsReady(); e != a { + t.Errorf("%q expected: %v got: %v", tc.name, e, a) + } + } } func TestServiceConditions(t *testing.T) { - svc := &Service{} - foo := &ServiceCondition { - Type: "Foo", - Status: "True", - } - bar := &ServiceCondition { - Type: "Bar", - Status: "True", - } + svc := &Service{} + foo := &ServiceCondition{ + Type: "Foo", + Status: "True", + } + bar := &ServiceCondition{ + Type: "Bar", + Status: "True", + } - // Add a single condition. - svc.Status.SetCondition(foo) - if got, want := len(svc.Status.Conditions), 1; got != want { - t.Fatalf("Unexpected Condition length; got %d, want %d", got, want) - } + // Add a single condition. + svc.Status.SetCondition(foo) + if got, want := len(svc.Status.Conditions), 1; got != want { + t.Fatalf("Unexpected Condition length; got %d, want %d", got, want) + } - // Remove non-existent condition. - svc.Status.RemoveCondition(bar.Type) - if got, want := len(svc.Status.Conditions), 1; got != want { - t.Fatalf("Unexpected Condition length; got %d, want %d", got, want) - } + // Remove non-existent condition. + svc.Status.RemoveCondition(bar.Type) + if got, want := len(svc.Status.Conditions), 1; got != want { + t.Fatalf("Unexpected Condition length; got %d, want %d", got, want) + } - // Add a second Condition. - svc.Status.SetCondition(bar) - if got, want := len(svc.Status.Conditions), 2; got != want { - t.Fatalf("Unexpected Condition length; got %d, want %d", got, want) - } + // Add a second Condition. + svc.Status.SetCondition(bar) + if got, want := len(svc.Status.Conditions), 2; got != want { + t.Fatalf("Unexpected Condition length; got %d, want %d", got, want) + } - // Remove the first Condition. - svc.Status.RemoveCondition(foo.Type) - if got, want := len(svc.Status.Conditions), 1; got != want { - t.Fatalf("Unexpected condition length; got %d, want %d", got, want) - } + // Remove the first Condition. + svc.Status.RemoveCondition(foo.Type) + if got, want := len(svc.Status.Conditions), 1; got != want { + t.Fatalf("Unexpected condition length; got %d, want %d", got, want) + } - // Test Add nil condition. - svc.Status.SetCondition(nil) - if got, want := len(svc.Status.Conditions), 1; got != want { - t.Fatal("Error, nil condition was allowed to be added.") - } + // Test Add nil condition. + svc.Status.SetCondition(nil) + if got, want := len(svc.Status.Conditions), 1; got != want { + t.Fatal("Error, nil condition was allowed to be added.") + } } diff --git a/pkg/controller/configuration/configuration.go b/pkg/controller/configuration/configuration.go index 0b1fb0147946..b5586cf5b06f 100644 --- a/pkg/controller/configuration/configuration.go +++ b/pkg/controller/configuration/configuration.go @@ -265,7 +265,7 @@ func (c *Controller) syncHandler(key string) error { if config.GetGeneration() == config.Status.ObservedGeneration { // TODO(vaikas): Check to see if Status.LatestCreatedRevisionName is ready and update Status.LatestReady glog.Infof("Skipping reconcile since already reconciled %d == %d", - config.Spec.Generation, config.Status.ObservedGeneration) + config.Generation, config.Status.ObservedGeneration) return nil } @@ -327,7 +327,7 @@ func (c *Controller) syncHandler(key string) error { if rev.ObjectMeta.Annotations == nil { rev.ObjectMeta.Annotations = make(map[string]string) } - rev.ObjectMeta.Annotations[ela.ConfigurationGenerationAnnotationKey] = fmt.Sprintf("%v", config.Spec.Generation) + rev.ObjectMeta.Annotations[ela.ConfigurationGenerationAnnotationKey] = fmt.Sprintf("%v", config.Generation) // Delete revisions when the parent Configuration is deleted. rev.OwnerReferences = append(rev.OwnerReferences, *controllerRef) @@ -348,7 +348,7 @@ func (c *Controller) syncHandler(key string) error { // Also update the LatestCreatedRevisionName so that we'll know revision to check // for ready state so that when ready, we can make it Latest. config.Status.LatestCreatedRevisionName = created.ObjectMeta.Name - config.Status.ObservedGeneration = config.Spec.Generation + config.Status.ObservedGeneration = config.Generation glog.Infof("Updating the configuration status:\n%+v", config) @@ -361,25 +361,12 @@ func (c *Controller) syncHandler(key string) error { } func generateRevisionName(u *v1alpha1.Configuration) (string, error) { - // TODO: consider making sure the length of the - // string will not cause problems down the stack - if u.Spec.Generation == 0 { - return "", fmt.Errorf("configuration generation cannot be 0") - } - return fmt.Sprintf("%s-%05d", u.Name, u.Spec.Generation), nil + return fmt.Sprintf("%s-%05d", u.Name, u.Generation), nil } func (c *Controller) updateStatus(u *v1alpha1.Configuration) (*v1alpha1.Configuration, error) { configClient := c.elaclientset.ElafrosV1alpha1().Configurations(u.Namespace) - newu, err := configClient.Get(u.Name, metav1.GetOptions{}) - if err != nil { - return nil, err - } - newu.Status = u.Status - - // TODO: for CRD there's no updatestatus, so use normal update - return configClient.Update(newu) - // return configClient.UpdateStatus(newu) + return configClient.UpdateStatus(u) } func (c *Controller) addRevisionEvent(obj interface{}) { diff --git a/pkg/controller/configuration/configuration_test.go b/pkg/controller/configuration/configuration_test.go index f6b01cc9ae2c..62e7c480265f 100644 --- a/pkg/controller/configuration/configuration_test.go +++ b/pkg/controller/configuration/configuration_test.go @@ -60,13 +60,12 @@ const ( func getTestConfiguration() *v1alpha1.Configuration { return &v1alpha1.Configuration{ ObjectMeta: metav1.ObjectMeta{ - SelfLink: "/apis/ela/v1alpha1/namespaces/test/configurations/test-config", - Name: "test-config", - Namespace: testNamespace, + SelfLink: "/apis/ela/v1alpha1/namespaces/test/configurations/test-config", + Name: "test-config", + Namespace: testNamespace, + Generation: 1, }, Spec: v1alpha1.ConfigurationSpec{ - //TODO(grantr): This is a workaround for generation initialization - Generation: 1, RevisionTemplate: v1alpha1.RevisionTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{ @@ -219,7 +218,7 @@ func TestCreateConfigurationsCreatesRevision(t *testing.T) { t.Errorf("rev does not have configuration label <%s:%s>", ela.ConfigurationLabelKey, config.Name) } - if rev.Annotations[ela.ConfigurationGenerationAnnotationKey] != fmt.Sprintf("%v", config.Spec.Generation) { + if rev.Annotations[ela.ConfigurationGenerationAnnotationKey] != fmt.Sprintf("%v", config.Generation) { t.Errorf("rev does not have generation annotation <%s:%s>", ela.ConfigurationGenerationAnnotationKey, config.Name) } diff --git a/pkg/controller/revision/revision.go b/pkg/controller/revision/revision.go index c78f27e1d948..998ed56d7324 100644 --- a/pkg/controller/revision/revision.go +++ b/pkg/controller/revision/revision.go @@ -1027,15 +1027,7 @@ func (c *Controller) removeFinalizers(rev *v1alpha1.Revision, ns string) error { func (c *Controller) updateStatus(rev *v1alpha1.Revision) (*v1alpha1.Revision, error) { prClient := c.elaclientset.ElafrosV1alpha1().Revisions(rev.Namespace) - newRev, err := prClient.Get(rev.Name, metav1.GetOptions{}) - if err != nil { - return nil, err - } - newRev.Status = rev.Status - - // TODO: for CRD there's no updatestatus, so use normal update - return prClient.Update(newRev) - // return prClient.UpdateStatus(newRev) + return prClient.UpdateStatus(rev) } // Given an endpoint see if it's managed by us and return the diff --git a/pkg/controller/route/BUILD.bazel b/pkg/controller/route/BUILD.bazel index 4c82da48d5a4..891d8d332f4d 100644 --- a/pkg/controller/route/BUILD.bazel +++ b/pkg/controller/route/BUILD.bazel @@ -15,6 +15,7 @@ go_library( "//pkg/apis/ela/v1alpha1:go_default_library", "//pkg/apis/istio/v1alpha2:go_default_library", "//pkg/client/clientset/versioned:go_default_library", + "//pkg/client/clientset/versioned/typed/ela/v1alpha1:go_default_library", "//pkg/client/informers/externalversions:go_default_library", "//pkg/client/listers/ela/v1alpha1:go_default_library", "//pkg/controller:go_default_library", @@ -23,10 +24,12 @@ go_library( "//vendor/go.opencensus.io/stats:go_default_library", "//vendor/go.opencensus.io/stats/view:go_default_library", "//vendor/go.opencensus.io/tag:go_default_library", + "//vendor/github.com/mattbaird/jsonpatch:go_default_library", "//vendor/k8s.io/api/core/v1:go_default_library", "//vendor/k8s.io/api/extensions/v1beta1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/types:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/intstr:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/runtime:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/wait:go_default_library", @@ -59,9 +62,11 @@ go_test( "//vendor/github.com/google/go-cmp/cmp:go_default_library", "//vendor/github.com/google/go-cmp/cmp/cmpopts:go_default_library", "//vendor/github.com/josephburnett/k8sflag/pkg/k8sflag:go_default_library", + "//vendor/github.com/mattbaird/jsonpatch:go_default_library", "//vendor/k8s.io/api/core/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/types:go_default_library", "//vendor/k8s.io/client-go/informers:go_default_library", "//vendor/k8s.io/client-go/kubernetes/fake:go_default_library", "//vendor/k8s.io/client-go/rest:go_default_library", diff --git a/pkg/controller/route/route.go b/pkg/controller/route/route.go index 87161b2e9f37..5c5f2e964b17 100644 --- a/pkg/controller/route/route.go +++ b/pkg/controller/route/route.go @@ -18,17 +18,21 @@ package route import ( "context" + "encoding/json" "errors" "fmt" "reflect" + "strings" "time" "github.com/golang/glog" "github.com/josephburnett/k8sflag/pkg/k8sflag" + "github.com/mattbaird/jsonpatch" corev1 "k8s.io/api/core/v1" v1beta1 "k8s.io/api/extensions/v1beta1" apierrs "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/wait" kubeinformers "k8s.io/client-go/informers" @@ -43,6 +47,7 @@ import ( "github.com/elafros/elafros/pkg/apis/ela" "github.com/elafros/elafros/pkg/apis/ela/v1alpha1" clientset "github.com/elafros/elafros/pkg/client/clientset/versioned" + elaclientset "github.com/elafros/elafros/pkg/client/clientset/versioned/typed/ela/v1alpha1" informers "github.com/elafros/elafros/pkg/client/informers/externalversions" listers "github.com/elafros/elafros/pkg/client/listers/ela/v1alpha1" "github.com/elafros/elafros/pkg/controller" @@ -584,7 +589,9 @@ func (c *Controller) extendConfigurationsWithIndirectTrafficTargets( } func (c *Controller) setLabelForGivenConfigurations( - route *v1alpha1.Route, configMap map[string]*v1alpha1.Configuration) error { + route *v1alpha1.Route, + configMap map[string]*v1alpha1.Configuration) error { + configClient := c.elaclientset.ElafrosV1alpha1().Configurations(route.Namespace) // Validate @@ -602,14 +609,13 @@ func (c *Controller) setLabelForGivenConfigurations( // Set label for newly added configurations as traffic target. for _, config := range configMap { - if config.Labels == nil { - config.Labels = make(map[string]string) - } else if _, ok := config.Labels[ela.RouteLabelKey]; ok { + if _, ok := config.Labels[ela.RouteLabelKey]; ok { continue } - config.Labels[ela.RouteLabelKey] = route.Name - if _, err := configClient.Update(config); err != nil { - glog.Errorf("Failed to update Configuration %s: %s", config.Name, err) + + err := c.setLabelForGivenConfiguration(configClient, route.Name, config.Name) + if err != nil { + glog.Errorf("Failed to patch Configuration %s: %s", config.Name, err) return err } } @@ -617,6 +623,31 @@ func (c *Controller) setLabelForGivenConfigurations( return nil } +func (c *Controller) setLabelForGivenConfiguration( + client elaclientset.ConfigurationInterface, + routeName, + configName string) error { + + pathPart := strings.Replace(ela.RouteLabelKey, "/", "~1", -1) + patch := []jsonpatch.JsonPatchOperation{ + { + Operation: "add", + Path: fmt.Sprintf("/metadata/labels/%s", pathPart), + Value: routeName, + }, + } + + jsonPatch, err := json.Marshal(patch) + if err != nil { + return err + } + + if _, err := client.Patch(configName, types.JSONPatchType, jsonPatch); err != nil { + return err + } + return nil +} + func (c *Controller) deleteLabelForOutsideOfGivenConfigurations( route *v1alpha1.Route, configMap map[string]*v1alpha1.Configuration) error { configClient := c.elaclientset.ElafrosV1alpha1().Configurations(route.Namespace) @@ -768,17 +799,7 @@ func (c *Controller) createOrUpdateRouteRules(route *v1alpha1.Route, configMap m func (c *Controller) updateStatus(route *v1alpha1.Route) (*v1alpha1.Route, error) { routeClient := c.elaclientset.ElafrosV1alpha1().Routes(route.Namespace) - existing, err := routeClient.Get(route.Name, metav1.GetOptions{}) - if err != nil { - return nil, err - } - // Check if there is anything to update. - if !reflect.DeepEqual(existing.Status, route.Status) { - existing.Status = route.Status - // TODO: for CRD there's no updatestatus, so use normal update. - return routeClient.Update(existing) - } - return existing, nil + return routeClient.UpdateStatus(route) } // consolidateTrafficTargets will consolidate all duplicate revisions @@ -942,7 +963,7 @@ func (c *Controller) updateIngressEvent(old, new interface{}) { Status: corev1.ConditionTrue, }) - if _, err = routeClient.Update(route); err != nil { + if _, err = routeClient.UpdateStatus(route); err != nil { glog.Errorf("Error updating readiness of route '%s/%s' upon ingress becoming: %v", ns, routeName, err) return diff --git a/pkg/controller/route/route_test.go b/pkg/controller/route/route_test.go index b7f9d0a8e10f..5860406ffcce 100644 --- a/pkg/controller/route/route_test.go +++ b/pkg/controller/route/route_test.go @@ -102,8 +102,6 @@ func getTestConfiguration() *v1alpha1.Configuration { Namespace: testNamespace, }, Spec: v1alpha1.ConfigurationSpec{ - // This is a workaround for generation initialization - Generation: 1, RevisionTemplate: v1alpha1.RevisionTemplateSpec{ Spec: v1alpha1.RevisionSpec{ Container: corev1.Container{ @@ -714,67 +712,54 @@ func TestCreateRouteDeletesOutdatedRouteRules(t *testing.T) { } } -func TestSetLabelToConfigurationDirectlyConfigured(t *testing.T) { - _, elaClient, controller, _, elaInformer := newTestController(t) +func TestRouteLabelIsSetOnConfiguration(t *testing.T) { config := getTestConfiguration() rev := getTestRevisionForConfig(config) - route := getTestRouteWithTrafficTargets( - []v1alpha1.TrafficTarget{ - v1alpha1.TrafficTarget{ - ConfigurationName: config.Name, - Percent: 100, - }, - }, - ) - - elaClient.ElafrosV1alpha1().Configurations(testNamespace).Create(config) - elaClient.ElafrosV1alpha1().Revisions(testNamespace).Create(rev) - elaClient.ElafrosV1alpha1().Routes(testNamespace).Create(route) - // Since updateRouteEvent looks in the lister, we need to add it to the informer - elaInformer.Elafros().V1alpha1().Routes().Informer().GetIndexer().Add(route) - controller.updateRouteEvent(KeyOrDie(route)) - - config, err := elaClient.ElafrosV1alpha1().Configurations(testNamespace).Get(config.Name, metav1.GetOptions{}) - if err != nil { - t.Fatalf("error getting config: %v", err) - } - - // Configuration should be labeled for this route - expectedLabels := map[string]string{ela.RouteLabelKey: route.Name} - if diff := cmp.Diff(expectedLabels, config.Labels); diff != "" { - t.Errorf("Unexpected label diff (-want +got): %v", diff) - } -} -func TestSetLabelToConfigurationIndirectlyConfigured(t *testing.T) { - _, elaClient, controller, _, elaInformer := newTestController(t) - config := getTestConfiguration() - rev := getTestRevisionForConfig(config) - route := getTestRouteWithTrafficTargets( - []v1alpha1.TrafficTarget{ - v1alpha1.TrafficTarget{ - RevisionName: rev.Name, - Percent: 100, - }, + testCases := []struct { + name string + trafficTarget v1alpha1.TrafficTarget + }{ + { + "configuration traffic target", + v1alpha1.TrafficTarget{ConfigurationName: config.Name, Percent: 100}, + }, + { + "revision traffic target", + v1alpha1.TrafficTarget{RevisionName: rev.Name, Percent: 100}, }, - ) - - elaClient.ElafrosV1alpha1().Configurations(testNamespace).Create(config) - elaClient.ElafrosV1alpha1().Revisions(testNamespace).Create(rev) - elaClient.ElafrosV1alpha1().Routes(testNamespace).Create(route) - // Since updateRouteEvent looks in the lister, we need to add it to the informer - elaInformer.Elafros().V1alpha1().Routes().Informer().GetIndexer().Add(route) - controller.updateRouteEvent(KeyOrDie(route)) - - config, err := elaClient.ElafrosV1alpha1().Configurations(testNamespace).Get(config.Name, metav1.GetOptions{}) - if err != nil { - t.Fatalf("error getting config: %v", err) } - // Configuration should be labeled for this route - expectedLabels := map[string]string{ela.RouteLabelKey: route.Name} - if diff := cmp.Diff(expectedLabels, config.Labels); diff != "" { - t.Errorf("Unexpected label diff (-want +got): %v", diff) + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + + route := getTestRouteWithTrafficTargets( + []v1alpha1.TrafficTarget{tc.trafficTarget}, + ) + + _, elaClient, controller, _, elaInformer := newTestController(t) + var patchAction kubetesting.PatchAction + reactionFunc := func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) { + patchAction = action.(kubetesting.PatchAction) + return + } + + elaClient.AddReactor("patch", "configurations", reactionFunc) + elaClient.ElafrosV1alpha1().Configurations(testNamespace).Create(config) + elaClient.ElafrosV1alpha1().Revisions(testNamespace).Create(rev) + elaClient.ElafrosV1alpha1().Routes(testNamespace).Create(route) + // Since updateRouteEvent looks in the lister, we need to add it to the informer + elaInformer.Elafros().V1alpha1().Routes().Informer().GetIndexer().Add(route) + controller.updateRouteEvent(KeyOrDie(route)) + + // Configuration should be labeled for this route + want := `[{"op":"add","path":"/metadata/labels/elafros.dev~1route","value":"test-route"}]` + got := string(patchAction.GetPatch()) + + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("Unexpected label diff (-want +got): %v", diff) + } + }) } } diff --git a/pkg/controller/service/service.go b/pkg/controller/service/service.go index d08911908a42..8edc061bb9c1 100644 --- a/pkg/controller/service/service.go +++ b/pkg/controller/service/service.go @@ -19,7 +19,6 @@ package service import ( "context" "fmt" - "reflect" "time" "github.com/golang/glog" @@ -299,17 +298,7 @@ func (c *Controller) updateServiceEvent(key string) error { func (c *Controller) updateStatus(service *v1alpha1.Service) (*v1alpha1.Service, error) { serviceClient := c.elaclientset.ElafrosV1alpha1().Services(service.Namespace) - existing, err := serviceClient.Get(service.Name, metav1.GetOptions{}) - if err != nil { - return nil, err - } - // Check if there is anything to update. - if !reflect.DeepEqual(existing.Status, service.Status) { - existing.Status = service.Status - // TODO: for CRD there's no updatestatus, so use normal update. - return serviceClient.Update(existing) - } - return existing, nil + return serviceClient.UpdateStatus(service) } func (c *Controller) reconcileConfiguration(config *v1alpha1.Configuration) error { diff --git a/pkg/webhook/BUILD.bazel b/pkg/webhook/BUILD.bazel index dfee72233fd3..31ca167d9adb 100644 --- a/pkg/webhook/BUILD.bazel +++ b/pkg/webhook/BUILD.bazel @@ -40,6 +40,7 @@ go_test( ], embed = [":go_default_library"], deps = [ + "//pkg/apis/build/v1alpha1:go_default_library", "//pkg/apis/ela/v1alpha1:go_default_library", "//vendor/github.com/mattbaird/jsonpatch:go_default_library", "//vendor/k8s.io/api/admission/v1beta1:go_default_library", diff --git a/pkg/webhook/configuration_test.go b/pkg/webhook/configuration_test.go index 18aea9c2a708..33e5984bc193 100644 --- a/pkg/webhook/configuration_test.go +++ b/pkg/webhook/configuration_test.go @@ -20,6 +20,7 @@ import ( "strings" "testing" + build "github.com/elafros/elafros/pkg/apis/build/v1alpha1" "github.com/elafros/elafros/pkg/apis/ela/v1alpha1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" @@ -27,7 +28,7 @@ import ( ) func TestValidConfigurationAllowed(t *testing.T) { - configuration := createConfiguration(testGeneration, testConfigurationName) + configuration := createConfiguration(testConfigurationName) if err := ValidateConfiguration(nil, &configuration, &configuration); err != nil { t.Fatalf("Expected allowed. Failed with %s", err) @@ -61,7 +62,7 @@ func TestEmptyTemplateInSpecNotAllowed(t *testing.T) { Name: testConfigurationName, }, Spec: v1alpha1.ConfigurationSpec{ - Generation: testGeneration, + Build: new(build.BuildSpec), RevisionTemplate: v1alpha1.RevisionTemplateSpec{}, }, } @@ -78,7 +79,6 @@ func TestEmptyContainerNotAllowed(t *testing.T) { Name: testConfigurationName, }, Spec: v1alpha1.ConfigurationSpec{ - Generation: testGeneration, RevisionTemplate: v1alpha1.RevisionTemplateSpec{ Spec: v1alpha1.RevisionSpec{ ServiceAccountName: "Fred", @@ -102,7 +102,6 @@ func TestServingStateNotAllowed(t *testing.T) { Name: testConfigurationName, }, Spec: v1alpha1.ConfigurationSpec{ - Generation: testGeneration, RevisionTemplate: v1alpha1.RevisionTemplateSpec{ Spec: v1alpha1.RevisionSpec{ ServingState: v1alpha1.RevisionServingStateActive, @@ -140,7 +139,6 @@ func TestUnwantedFieldInContainerNotAllowed(t *testing.T) { Name: testConfigurationName, }, Spec: v1alpha1.ConfigurationSpec{ - Generation: testGeneration, RevisionTemplate: v1alpha1.RevisionTemplateSpec{ Spec: v1alpha1.RevisionSpec{ Container: container, diff --git a/pkg/webhook/route_test.go b/pkg/webhook/route_test.go index b0fdbdf94fb7..85cee708f658 100644 --- a/pkg/webhook/route_test.go +++ b/pkg/webhook/route_test.go @@ -29,8 +29,7 @@ func createRouteWithTraffic(trafficTargets []v1alpha1.TrafficTarget) v1alpha1.Ro Name: testRouteName, }, Spec: v1alpha1.RouteSpec{ - Generation: testGeneration, - Traffic: trafficTargets, + Traffic: trafficTargets, }, } } diff --git a/pkg/webhook/service_test.go b/pkg/webhook/service_test.go index 392805dbd523..950a09d9c025 100644 --- a/pkg/webhook/service_test.go +++ b/pkg/webhook/service_test.go @@ -38,7 +38,7 @@ func TestRunLatest(t *testing.T) { s := v1alpha1.Service{ Spec: v1alpha1.ServiceSpec{ RunLatest: &v1alpha1.RunLatestType{ - Configuration: createConfiguration(1, "config").Spec, + Configuration: createConfiguration("config").Spec, }, }, } @@ -68,7 +68,7 @@ func TestPinned(t *testing.T) { Spec: v1alpha1.ServiceSpec{ Pinned: &v1alpha1.PinnedType{ RevisionName: "revision", - Configuration: createConfiguration(1, "config").Spec, + Configuration: createConfiguration("config").Spec, }, }, } diff --git a/pkg/webhook/webhook.go b/pkg/webhook/webhook.go index 6cca16cbcd55..a35987ebab3c 100644 --- a/pkg/webhook/webhook.go +++ b/pkg/webhook/webhook.go @@ -414,13 +414,11 @@ func (ac *AdmissionController) admit(request *admissionv1beta1.AdmissionRequest) } glog.Infof("Kind: %q PatchBytes: %v", request.Kind, string(patchBytes)) + patchType := admissionv1beta1.PatchTypeJSONPatch return &admissionv1beta1.AdmissionResponse{ - Patch: patchBytes, - Allowed: true, - PatchType: func() *admissionv1beta1.PatchType { - pt := admissionv1beta1.PatchTypeJSONPatch - return &pt - }(), + Patch: patchBytes, + Allowed: true, + PatchType: &patchType, } } @@ -458,12 +456,6 @@ func (ac *AdmissionController) mutate(kind string, oldBytes []byte, newBytes []b var patches []jsonpatch.JsonPatchOperation - err := updateGeneration(&patches, oldObj, newObj) - if err != nil { - glog.Warningf("Failed to update generation : %s", err) - return nil, fmt.Errorf("Failed to update generation: %s", err) - } - if defaulter := handler.Defaulter; defaulter != nil { if err := defaulter(&patches, oldObj, newObj); err != nil { glog.Warningf("Failed the resource specific defaulter: %s", err) @@ -484,6 +476,7 @@ func (ac *AdmissionController) mutate(kind string, oldBytes []byte, newBytes []b glog.Warningf("Failed to validate : %s", err) return nil, fmt.Errorf("Failed to validate: %s", err) } + return json.Marshal(patches) } @@ -500,63 +493,6 @@ func validateMetadata(new GenericCRD) error { return nil } -// updateGeneration sets the generation by following this logic: -// if there's no old object, it's create, set generation to 1 -// if there's an old object and spec has changed, set generation to oldGeneration + 1 -// appends the patch to patches if changes are necessary. -// TODO: Generation does not work correctly with CRD. They are scrubbed -// by the APIserver (https://github.com/kubernetes/kubernetes/issues/58778) -// So, we add Generation here. Once that gets fixed, remove this and use -// ObjectMeta.Generation instead. -func updateGeneration(patches *[]jsonpatch.JsonPatchOperation, old GenericCRD, new GenericCRD) error { - var oldGeneration int64 - if old == nil { - glog.Infof("Old is nil") - } else { - oldGeneration = old.GetGeneration() - } - if oldGeneration == 0 { - glog.Infof("Creating an object, setting generation to 1") - *patches = append(*patches, jsonpatch.JsonPatchOperation{ - Operation: "add", - Path: "/spec/generation", - Value: 1, - }) - return nil - } - - oldSpecJSON, err := old.GetSpecJSON() - if err != nil { - glog.Warningf("Failed to get Spec JSON for old: %s", err) - } - newSpecJSON, err := new.GetSpecJSON() - if err != nil { - glog.Warningf("Failed to get Spec JSON for new: %s", err) - } - - specPatches, err := jsonpatch.CreatePatch(oldSpecJSON, newSpecJSON) - if err != nil { - fmt.Printf("Error creating JSON patch:%v", err) - return err - } - if len(specPatches) > 0 { - specPatchesJSON, err := json.Marshal(specPatches) - if err != nil { - glog.Infof("Failed to marshal spec patches: %s", err) - return err - } - glog.Infof("Specs differ:\n%+v\n", string(specPatchesJSON)) - *patches = append(*patches, jsonpatch.JsonPatchOperation{ - Operation: "replace", - Path: "/spec/generation", - Value: oldGeneration + 1, - }) - return nil - } - glog.Infof("No changes in the spec, not bumping generation...") - return nil -} - func generateSecret(name, namespace string) (*corev1.Secret, error) { serverKey, serverCert, caCert, err := CreateCerts() if err != nil { diff --git a/pkg/webhook/webhook_test.go b/pkg/webhook/webhook_test.go index aade3500ee0f..e1dafbf7f7ce 100644 --- a/pkg/webhook/webhook_test.go +++ b/pkg/webhook/webhook_test.go @@ -48,7 +48,6 @@ const ( imageName = "test-container-image" envVarName = "envname" envVarValue = "envvalue" - testGeneration = 1 testRouteName = "test-route-name" testRevisionName = "test-revision" testServiceName = "test-service-name" @@ -132,7 +131,7 @@ func TestInvalidNewConfigurationNameFails(t *testing.T) { Kind: metav1.GroupVersionKind{Kind: "Configuration"}, } invalidName := "configuration.example" - config := createConfiguration(0, invalidName) + config := createConfiguration(invalidName) marshaled, err := json.Marshal(config) if err != nil { t.Fatalf("Failed to marshal configuration: %s", err) @@ -141,7 +140,7 @@ func TestInvalidNewConfigurationNameFails(t *testing.T) { expectFailsWith(t, ac.admit(req), "Invalid resource name") invalidName = strings.Repeat("a", 64) - config = createConfiguration(0, invalidName) + config = createConfiguration(invalidName) marshaled, err = json.Marshal(config) if err != nil { t.Fatalf("Failed to marshal configuration: %s", err) @@ -154,14 +153,13 @@ func TestValidNewConfigurationObject(t *testing.T) { _, ac := newNonRunningTestAdmissionController(t, newDefaultOptions()) resp := ac.admit(createValidCreateConfiguration()) expectAllowed(t, resp) - p := incrementGenerationPatch(0) - expectPatches(t, resp.Patch, []jsonpatch.JsonPatchOperation{p}) + expectPatches(t, resp.Patch, []jsonpatch.JsonPatchOperation{}) } func TestValidConfigurationNoChanges(t *testing.T) { _, ac := newNonRunningTestAdmissionController(t, newDefaultOptions()) - old := createConfiguration(testGeneration, testConfigurationName) - new := createConfiguration(testGeneration, testConfigurationName) + old := createConfiguration(testConfigurationName) + new := createConfiguration(testConfigurationName) resp := ac.admit(createUpdateConfiguration(&old, &new)) expectAllowed(t, resp) expectPatches(t, resp.Patch, []jsonpatch.JsonPatchOperation{}) @@ -169,8 +167,8 @@ func TestValidConfigurationNoChanges(t *testing.T) { func TestValidConfigurationEnvChanges(t *testing.T) { _, ac := newNonRunningTestAdmissionController(t, newDefaultOptions()) - old := createConfiguration(testGeneration, testConfigurationName) - new := createConfiguration(testGeneration, testConfigurationName) + old := createConfiguration(testConfigurationName) + new := createConfiguration(testConfigurationName) new.Spec.RevisionTemplate.Spec.Container.Env = []corev1.EnvVar{ corev1.EnvVar{ Name: envVarName, @@ -179,13 +177,7 @@ func TestValidConfigurationEnvChanges(t *testing.T) { } resp := ac.admit(createUpdateConfiguration(&old, &new)) expectAllowed(t, resp) - expectPatches(t, resp.Patch, []jsonpatch.JsonPatchOperation{ - jsonpatch.JsonPatchOperation{ - Operation: "replace", - Path: "/spec/generation", - Value: 2, - }, - }) + expectPatches(t, resp.Patch, []jsonpatch.JsonPatchOperation{}) } func TestInvalidNewRouteNameFails(t *testing.T) { @@ -195,7 +187,7 @@ func TestInvalidNewRouteNameFails(t *testing.T) { Kind: metav1.GroupVersionKind{Kind: "Route"}, } invalidName := "route.example" - config := createRoute(0, invalidName) + config := createRoute(invalidName) marshaled, err := json.Marshal(config) if err != nil { t.Fatalf("Failed to marshal route: %s", err) @@ -204,7 +196,7 @@ func TestInvalidNewRouteNameFails(t *testing.T) { expectFailsWith(t, ac.admit(req), "Invalid resource name") invalidName = strings.Repeat("a", 64) - config = createRoute(0, invalidName) + config = createRoute(invalidName) marshaled, err = json.Marshal(config) if err != nil { t.Fatalf("Failed to marshal route: %s", err) @@ -217,14 +209,13 @@ func TestValidNewRouteObject(t *testing.T) { _, ac := newNonRunningTestAdmissionController(t, newDefaultOptions()) resp := ac.admit(createValidCreateRoute()) expectAllowed(t, resp) - p := incrementGenerationPatch(0) - expectPatches(t, resp.Patch, []jsonpatch.JsonPatchOperation{p}) + expectPatches(t, resp.Patch, []jsonpatch.JsonPatchOperation{}) } func TestValidRouteNoChanges(t *testing.T) { _, ac := newNonRunningTestAdmissionController(t, newDefaultOptions()) - old := createRoute(1, testRouteName) - new := createRoute(1, testRouteName) + old := createRoute(testRouteName) + new := createRoute(testRouteName) resp := ac.admit(createUpdateRoute(&old, &new)) expectAllowed(t, resp) expectPatches(t, resp.Patch, []jsonpatch.JsonPatchOperation{}) @@ -232,7 +223,7 @@ func TestValidRouteNoChanges(t *testing.T) { func TestInvalidOldRoute(t *testing.T) { _, ac := newNonRunningTestAdmissionController(t, newDefaultOptions()) - new := createRoute(1, testRouteName) + new := createRoute(testRouteName) newBytes, err := json.Marshal(new) if err != nil { t.Errorf("Marshal(%v) = %v", new, err) @@ -244,7 +235,7 @@ func TestInvalidOldRoute(t *testing.T) { func TestInvalidNewRoute(t *testing.T) { _, ac := newNonRunningTestAdmissionController(t, newDefaultOptions()) - old := createRoute(1, testRouteName) + old := createRoute(testRouteName) oldBytes, err := json.Marshal(old) if err != nil { t.Errorf("Marshal(%v) = %v", old, err) @@ -256,8 +247,8 @@ func TestInvalidNewRoute(t *testing.T) { func TestValidRouteChanges(t *testing.T) { _, ac := newNonRunningTestAdmissionController(t, newDefaultOptions()) - old := createRoute(1, testRouteName) - new := createRoute(1, testRouteName) + old := createRoute(testRouteName) + new := createRoute(testRouteName) new.Spec.Traffic = []v1alpha1.TrafficTarget{ v1alpha1.TrafficTarget{ RevisionName: testRevisionName, @@ -266,13 +257,7 @@ func TestValidRouteChanges(t *testing.T) { } resp := ac.admit(createUpdateRoute(&old, &new)) expectAllowed(t, resp) - expectPatches(t, resp.Patch, []jsonpatch.JsonPatchOperation{ - jsonpatch.JsonPatchOperation{ - Operation: "replace", - Path: "/spec/generation", - Value: 2, - }, - }) + expectPatches(t, resp.Patch, []jsonpatch.JsonPatchOperation{}) } func TestValidNewRevisionObject(t *testing.T) { @@ -291,11 +276,6 @@ func TestValidNewRevisionObject(t *testing.T) { resp := ac.admit(req) expectAllowed(t, resp) expectPatches(t, resp.Patch, []jsonpatch.JsonPatchOperation{ - jsonpatch.JsonPatchOperation{ - Operation: "add", - Path: "/spec/generation", - Value: 1, - }, jsonpatch.JsonPatchOperation{ Operation: "add", Path: "/spec/servingState", @@ -328,13 +308,7 @@ func TestValidRevisionUpdates(t *testing.T) { req.Object.Raw = marshaled resp := ac.admit(req) expectAllowed(t, resp) - expectPatches(t, resp.Patch, []jsonpatch.JsonPatchOperation{ - jsonpatch.JsonPatchOperation{ - Operation: "add", - Path: "/spec/generation", - Value: 1, - }, - }) + expectPatches(t, resp.Patch, []jsonpatch.JsonPatchOperation{}) } func TestInvalidRevisionUpdate(t *testing.T) { @@ -394,28 +368,26 @@ func TestValidNewServicePinned(t *testing.T) { _, ac := newNonRunningTestAdmissionController(t, newDefaultOptions()) resp := ac.admit(createValidCreateServicePinned()) expectAllowed(t, resp) - p := incrementGenerationPatch(0) - expectPatches(t, resp.Patch, []jsonpatch.JsonPatchOperation{p}) + expectPatches(t, resp.Patch, []jsonpatch.JsonPatchOperation{}) } func TestValidNewServiceRunLatest(t *testing.T) { _, ac := newNonRunningTestAdmissionController(t, newDefaultOptions()) resp := ac.admit(createValidCreateServiceRunLatest()) expectAllowed(t, resp) - p := incrementGenerationPatch(0) - expectPatches(t, resp.Patch, []jsonpatch.JsonPatchOperation{p}) + expectPatches(t, resp.Patch, []jsonpatch.JsonPatchOperation{}) } func TestInvalidNewServiceNoSpecs(t *testing.T) { _, ac := newNonRunningTestAdmissionController(t, newDefaultOptions()) - svc := createServicePinned(0, testServiceName) + svc := createServicePinned(testServiceName) svc.Spec.Pinned = nil expectFailsWith(t, ac.admit(createCreateService(svc)), "exactly one of runLatest or pinned") } func TestInvalidNewServiceNoRevisionNameInPinned(t *testing.T) { _, ac := newNonRunningTestAdmissionController(t, newDefaultOptions()) - svc := createServicePinned(0, testServiceName) + svc := createServicePinned(testServiceName) svc.Spec.Pinned.RevisionName = "" expectFailsWith(t, ac.admit(createCreateService(svc)), "spec.pinned.revisionName") } @@ -474,7 +446,7 @@ func createValidCreateConfiguration() *admissionv1beta1.AdmissionRequest { Operation: admissionv1beta1.Create, Kind: metav1.GroupVersionKind{Kind: "Configuration"}, } - config := createConfiguration(0, testConfigurationName) + config := createConfiguration(testConfigurationName) marshaled, err := json.Marshal(config) if err != nil { panic("failed to marshal configuration") @@ -495,7 +467,7 @@ func createValidCreateRoute() *admissionv1beta1.AdmissionRequest { Operation: admissionv1beta1.Create, Kind: metav1.GroupVersionKind{Kind: "Route"}, } - route := createRoute(0, testRouteName) + route := createRoute(testRouteName) marshaled, err := json.Marshal(route) if err != nil { panic("failed to marshal route") @@ -554,11 +526,11 @@ func createCreateService(service v1alpha1.Service) *admissionv1beta1.AdmissionRe } func createValidCreateServicePinned() *admissionv1beta1.AdmissionRequest { - return createCreateService(createServicePinned(0, testServiceName)) + return createCreateService(createServicePinned(testServiceName)) } func createValidCreateServiceRunLatest() *admissionv1beta1.AdmissionRequest { - return createCreateService(createServiceRunLatest(0, testServiceName)) + return createCreateService(createServiceRunLatest(testServiceName)) } func createWebhook(ac *AdmissionController, webhook *admissionregistrationv1beta1.MutatingWebhookConfiguration) { @@ -620,14 +592,13 @@ func expectPatches(t *testing.T, a []byte, e []jsonpatch.JsonPatchOperation) { } } -func createConfiguration(generation int64, configurationName string) v1alpha1.Configuration { +func createConfiguration(configurationName string) v1alpha1.Configuration { return v1alpha1.Configuration{ ObjectMeta: metav1.ObjectMeta{ Namespace: testNamespace, Name: configurationName, }, Spec: v1alpha1.ConfigurationSpec{ - Generation: generation, RevisionTemplate: v1alpha1.RevisionTemplateSpec{ Spec: v1alpha1.RevisionSpec{ Container: corev1.Container{ @@ -643,14 +614,13 @@ func createConfiguration(generation int64, configurationName string) v1alpha1.Co } } -func createRoute(generation int64, routeName string) v1alpha1.Route { +func createRoute(routeName string) v1alpha1.Route { return v1alpha1.Route{ ObjectMeta: metav1.ObjectMeta{ Namespace: testNamespace, Name: routeName, }, Spec: v1alpha1.RouteSpec{ - Generation: generation, Traffic: []v1alpha1.TrafficTarget{ v1alpha1.TrafficTarget{ Name: "test-traffic-target", @@ -676,41 +646,31 @@ func createRevision(revName string) v1alpha1.Revision { } } -func createServicePinned(generation int64, serviceName string) v1alpha1.Service { +func createServicePinned(serviceName string) v1alpha1.Service { return v1alpha1.Service{ ObjectMeta: metav1.ObjectMeta{ Namespace: testNamespace, Name: serviceName, }, Spec: v1alpha1.ServiceSpec{ - Generation: generation, Pinned: &v1alpha1.PinnedType{ RevisionName: testRevisionName, - Configuration: createConfiguration(generation, "config").Spec, + Configuration: createConfiguration("config").Spec, }, }, } } -func createServiceRunLatest(generation int64, serviceName string) v1alpha1.Service { +func createServiceRunLatest(serviceName string) v1alpha1.Service { return v1alpha1.Service{ ObjectMeta: metav1.ObjectMeta{ Namespace: testNamespace, Name: serviceName, }, Spec: v1alpha1.ServiceSpec{ - Generation: generation, RunLatest: &v1alpha1.RunLatestType{ - Configuration: createConfiguration(generation, "config").Spec, + Configuration: createConfiguration("config").Spec, }, }, } } - -func incrementGenerationPatch(old int64) jsonpatch.JsonPatchOperation { - return jsonpatch.JsonPatchOperation{ - Operation: "add", - Path: "/spec/generation", - Value: old + 1, - } -} diff --git a/revision.yaml b/revision.yaml index 7a7780fa0072..5d2fcd4e8cc8 100644 --- a/revision.yaml +++ b/revision.yaml @@ -23,3 +23,5 @@ spec: kind: Revision plural: revisions scope: Namespaced + subresources: + status: {} diff --git a/route.yaml b/route.yaml index 5d1a10cb7cc6..d15e9e8f1652 100644 --- a/route.yaml +++ b/route.yaml @@ -23,3 +23,5 @@ spec: kind: Route plural: routes scope: Namespaced + subresources: + status: {} diff --git a/service.yaml b/service.yaml index 2eb9e77daaa3..98bb200d5c52 100644 --- a/service.yaml +++ b/service.yaml @@ -23,3 +23,5 @@ spec: kind: Service plural: services scope: Namespaced + subresources: + status: {} diff --git a/test/conformance/route_test.go b/test/conformance/route_test.go index e122372e0f05..7c0d9be59b14 100644 --- a/test/conformance/route_test.go +++ b/test/conformance/route_test.go @@ -250,8 +250,7 @@ var _ = Describe("Route", func() { newConfig, err := configClient.Patch(configName, types.JSONPatchType, patchBytes, "") Expect(err).NotTo(HaveOccurred()) - Expect(newConfig.Generation).To(Equal(int64(0))) - Expect(newConfig.Spec.Generation).To(Equal(int64(2))) + Expect(newConfig.Generation).To(Equal(int64(2))) By("A new Revision will be made and the Configuration will be updated with it") var newRevisionName string