Enable the status subresource (knative#516)

* Enable the status subresource

- template CRDs are include in this change so their metadata.generation
  increments
- the build controller now uses the /status endpoint on update

* fix comment typos

config/200-clusterrole.yaml

@@ -18,6 +18,11 @@ rules:
   - apiGroups: ["build.knative.dev"]
     resources: ["builds", "buildtemplates", "clusterbuildtemplates"]
     verbs: ["get", "list", "create", "update", "delete", "patch", "watch"]
+  # We enable the status subresource on the templates CRDs so metadata.generation
+  # bumping will work in 1.11
+  - apiGroups: ["build.knative.dev"]
+    resources: ["builds/status", "buildtemplates/status", "clusterbuildtemplates/status"]
+    verbs: ["get", "list", "create", "update", "delete", "patch", "watch"]
   - apiGroups: ["caching.internal.knative.dev"]
     resources: ["images"]
     verbs: ["get", "list", "create", "update", "delete", "deletecollection", "patch", "watch"]

config/300-build.yaml

@@ -25,6 +25,8 @@ spec:
     - all
     - knative
   scope: Namespaced
+  subresources:
+    status: {}
   additionalPrinterColumns:
   - name: Succeeded
     type: string

config/300-buildtemplate.yaml

@@ -25,6 +25,10 @@ spec:
     - all
     - knative
   scope: Namespaced
+  # Enable the status subresource so we start bumping
+  # metadata.generation properly
+  subresources:
+    status: {}
   additionalPrinterColumns:
   - name: Age
     type: date

config/300-clusterbuildtemplate.yaml

@@ -25,6 +25,10 @@ spec:
     - all
     - knative
   scope: Cluster
+  # Enable the status subresource so we start bumping
+  # metadata.generation properly
+  subresources:
+    status: {}
   additionalPrinterColumns:
   - name: Age
     type: date

pkg/apis/build/v1alpha1/build_template_types.go

@@ -54,12 +54,11 @@ var _ apis.Defaultable = (*BuildTemplate)(nil)
 
 // BuildTemplateSpec is the spec for a BuildTemplate.
 type BuildTemplateSpec 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.
+	// TODO(dprotaso) Metadata.Generation should increment so we
+	// can drop this property when conversion webhooks enable us
+	// to migrate
 	// +optional
-	Generation int64 `json:"generation,omitempty"`
+	DeprecatedGeneration int64 `json:"generation,omitempty"`
 
 	// Parameters defines the parameters that can be populated in a template.
 	Parameters []ParameterSpec `json:"parameters,omitempty"`

pkg/apis/build/v1alpha1/build_template_types_test.go

@@ -47,15 +47,3 @@ func TestBuildTemplateGroupVersionKind(t *testing.T) {
 		t.Errorf("GetGroupVersionKind mismatch; expected: %v got: %v", expectedKind, c.GetGroupVersionKind().Kind)
 	}
 }
-
-func TestBuildTemplateGeneration(t *testing.T) {
-	c := BuildTemplate{}
-	if a := c.GetGeneration(); a != 0 {
-		t.Errorf("empty build template generation should be 0 but got: %d", a)
-	}
-
-	c.SetGeneration(5)
-	if e, a := int64(5), c.GetGeneration(); e != a {
-		t.Errorf("getgeneration mismatch; expected: %d got: %d", e, a)
-	}
-}

pkg/apis/build/v1alpha1/build_types.go

@@ -27,7 +27,6 @@ import (
 )
 
 // +genclient
-// +genclient:noStatus
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
 
 // Build represents a build of a container image. A Build is made up of a
@@ -50,12 +49,11 @@ var _ apis.Defaultable = (*Build)(nil)
 
 // BuildSpec is the spec for a Build resource.
 type BuildSpec 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.
+	// TODO(dprotaso) Metadata.Generation should increment so we
+	// can drop this property when conversion webhooks enable us
+	// to migrate
 	// +optional
-	Generation int64 `json:"generation,omitempty"`
+	DeprecatedGeneration int64 `json:"generation,omitempty"`
 
 	// Source specifies the input to the build.
 	// +optional

pkg/apis/build/v1alpha1/build_types_test.go

@@ -69,18 +69,6 @@ func TestBuildConditions(t *testing.T) {
 	}
 }
 
-func TestBuildGeneration(t *testing.T) {
-	b := Build{}
-	if a := b.GetGeneration(); a != 0 {
-		t.Errorf("empty build generation should be 0 but got: %d", a)
-	}
-
-	b.SetGeneration(5)
-	if e, a := int64(5), b.GetGeneration(); e != a {
-		t.Errorf("getgeneration mismatch; expected: %d got: %d", e, a)
-	}
-}
-
 func TestBuildGroupVersionKind(t *testing.T) {
 	b := Build{}
 

pkg/apis/build/v1alpha1/cluster_build_template_types_test.go

@@ -23,19 +23,6 @@ import (
 	corev1 "k8s.io/api/core/v1"
 )
 
-func TestGeneration(t *testing.T) {
-	c := ClusterBuildTemplate{}
-	if a := c.GetGeneration(); a != 0 {
-		t.Errorf("empty cluster build template generation should be 0 but got: %d", a)
-	}
-
-	c.SetGeneration(5)
-	if e, a := int64(5), c.GetGeneration(); e != a {
-		t.Errorf("getgeneration mismatch; expected: %d got: %d", e, a)
-	}
-
-}
-
 func TestBuildSpec(t *testing.T) {
 	c := ClusterBuildTemplate{
 		Spec: BuildTemplateSpec{

pkg/reconciler/build/build.go

@@ -212,11 +212,7 @@ func (c *Reconciler) updateStatus(u *v1alpha1.Build) error {
 	}
 	newb.Status = u.Status
 
-	// Until #38113 is merged, we must use Update instead of UpdateStatus to
-	// update the Status block of the Build resource. UpdateStatus will not
-	// allow changes to the Spec of the resource, which is ideal for ensuring
-	// nothing other than resource status has been updated.
-	_, err = c.buildclientset.BuildV1alpha1().Builds(u.Namespace).Update(newb)
+	_, err = c.buildclientset.BuildV1alpha1().Builds(u.Namespace).UpdateStatus(newb)
 	return err
 }