Generation and separate status updates for DCs

api/swagger-spec/oapi-v1.json

@@ -5529,6 +5529,66 @@
      }
     ]
    },
+   {
+    "path": "/oapi/v1/namespaces/{namespace}/deploymentconfigs/{name}/status",
+    "description": "OpenShift REST API, version v1",
+    "operations": [
+     {
+      "type": "v1.DeploymentConfig",
+      "method": "PUT",
+      "summary": "replace status of the specified DeploymentConfig",
+      "nickname": "replaceNamespacedDeploymentConfigStatus",
+      "parameters": [
+       {
+        "type": "string",
+        "paramType": "query",
+        "name": "pretty",
+        "description": "If 'true', then the output is pretty printed.",
+        "required": false,
+        "allowMultiple": false
+       },
+       {
+        "type": "v1.DeploymentConfig",
+        "paramType": "body",
+        "name": "body",
+        "description": "",
+        "required": true,
+        "allowMultiple": false
+       },
+       {
+        "type": "string",
+        "paramType": "path",
+        "name": "namespace",
+        "description": "object name and auth scope, such as for teams and projects",
+        "required": true,
+        "allowMultiple": false
+       },
+       {
+        "type": "string",
+        "paramType": "path",
+        "name": "name",
+        "description": "name of the DeploymentConfig",
+        "required": true,
+        "allowMultiple": false
+       }
+      ],
+      "responseMessages": [
+       {
+        "code": 200,
+        "message": "OK",
+        "responseModel": "v1.DeploymentConfig"
+       }
+      ],
+      "produces": [
+       "application/json",
+       "application/yaml"
+      ],
+      "consumes": [
+       "*/*"
+      ]
+     }
+    ]
+   },
    {
     "path": "/oapi/v1/namespaces/{namespace}/generatedeploymentconfigs/{name}",
     "description": "OpenShift REST API, version v1",
@@ -20531,6 +20591,11 @@
      "details": {
       "$ref": "v1.DeploymentDetails",
       "description": "Details are the reasons for the update to this deployment config. This could be based on a change made by the user or caused by an automatic trigger"
+     },
+     "observedGeneration": {
+      "type": "integer",
+      "format": "int64",
+      "description": "ObservedGeneration is the most recent generation observed by the controller."
      }
     }
    },

pkg/api/v1beta3/deep_copy_generated.go

@@ -1611,6 +1611,7 @@ func deepCopy_v1beta3_DeploymentConfigStatus(in deployapiv1beta3.DeploymentConfi
 	} else {
 		out.Details = nil
 	}
+	out.ObservedGeneration = in.ObservedGeneration
 	return nil
 }
 

pkg/authorization/api/types.go

@@ -94,7 +94,7 @@ var (
 		OpenshiftAllGroupName: {OpenshiftExposedGroupName, UserGroupName, OAuthGroupName, PolicyOwnerGroupName, SDNGroupName, PermissionGrantingGroupName, OpenshiftStatusGroupName, "projects",
 			"clusterroles", "clusterrolebindings", "clusterpolicies", "clusterpolicybindings", "images" /* cluster scoped*/, "projectrequests", "builds/details", "imagestreams/secrets",
 			"selfsubjectrulesreviews"},
-		OpenshiftStatusGroupName: {"imagestreams/status", "routes/status"},
+		OpenshiftStatusGroupName: {"imagestreams/status", "routes/status", "deploymentconfigs/status"},
 
 		QuotaGroupName:         {"limitranges", "resourcequotas", "resourcequotausages"},
 		KubeExposedGroupName:   {"pods", "replicationcontrollers", "serviceaccounts", "services", "endpoints", "persistentvolumeclaims", "pods/log", "configmaps"},

pkg/client/deploymentconfigs.go

@@ -27,6 +27,7 @@ type DeploymentConfigInterface interface {
 	Rollback(config *deployapi.DeploymentConfigRollback) (*deployapi.DeploymentConfig, error)
 	GetScale(name string) (*extensions.Scale, error)
 	UpdateScale(scale *extensions.Scale) (*extensions.Scale, error)
+	UpdateStatus(config *deployapi.DeploymentConfig) (*deployapi.DeploymentConfig, error)
 }
 
 // deploymentConfigs implements DeploymentConfigsNamespacer interface
@@ -98,6 +99,7 @@ func (c *deploymentConfigs) Generate(name string) (result *deployapi.DeploymentC
 	return
 }
 
+// Rollback rolls a deploymentConfig back to a previous configuration
 func (c *deploymentConfigs) Rollback(config *deployapi.DeploymentConfigRollback) (result *deployapi.DeploymentConfig, err error) {
 	result = &deployapi.DeploymentConfig{}
 	err = c.r.Post().
@@ -109,14 +111,14 @@ func (c *deploymentConfigs) Rollback(config *deployapi.DeploymentConfigRollback)
 	return
 }
 
-// Get returns information about a particular deploymentConfig
+// GetScale returns information about a particular deploymentConfig via its scale subresource
 func (c *deploymentConfigs) GetScale(name string) (result *extensions.Scale, err error) {
 	result = &extensions.Scale{}
 	err = c.r.Get().Namespace(c.ns).Resource("deploymentConfigs").Name(name).SubResource("scale").Do().Into(result)
 	return
 }
 
-// Update updates an existing deploymentConfig
+// UpdateScale scales an existing deploymentConfig via its scale subresource
 func (c *deploymentConfigs) UpdateScale(scale *extensions.Scale) (result *extensions.Scale, err error) {
 	result = &extensions.Scale{}
 
@@ -129,3 +131,10 @@ func (c *deploymentConfigs) UpdateScale(scale *extensions.Scale) (result *extens
 	err = c.r.Put().Namespace(c.ns).Resource("deploymentConfigs").Name(scale.Name).SubResource("scale").Body(encodedBytes).Do().Into(result)
 	return
 }
+
+// UpdateStatus updates the status for an existing deploymentConfig.
+func (c *deploymentConfigs) UpdateStatus(deploymentConfig *deployapi.DeploymentConfig) (result *deployapi.DeploymentConfig, err error) {
+	result = &deployapi.DeploymentConfig{}
+	err = c.r.Put().Namespace(c.ns).Resource("deploymentConfigs").Name(deploymentConfig.Name).SubResource("status").Body(deploymentConfig).Do().Into(result)
+	return
+}

pkg/client/testclient/fake_deploymentconfigs.go

@@ -63,7 +63,6 @@ func (c *FakeDeploymentConfigs) Watch(opts kapi.ListOptions) (watch.Interface, e
 
 func (c *FakeDeploymentConfigs) Generate(name string) (*deployapi.DeploymentConfig, error) {
 	obj, err := c.Fake.Invokes(ktestclient.NewGetAction("generatedeploymentconfigs", c.Namespace, name), &deployapi.DeploymentConfig{})
-
 	if obj == nil {
 		return nil, err
 	}
@@ -97,3 +96,12 @@ func (c *FakeDeploymentConfigs) UpdateScale(inObj *extensions.Scale) (*extension
 
 	return obj.(*extensions.Scale), err
 }
+
+func (c *FakeDeploymentConfigs) UpdateStatus(inObj *deployapi.DeploymentConfig) (*deployapi.DeploymentConfig, error) {
+	obj, err := c.Fake.Invokes(ktestclient.NewUpdateAction("deploymentconfigs/status", c.Namespace, inObj), inObj)
+	if obj == nil {
+		return nil, err
+	}
+
+	return obj.(*deployapi.DeploymentConfig), err
+}

pkg/cmd/cli/cmd/deploy.go

@@ -211,6 +211,11 @@ func (o DeployOptions) RunDeploy() error {
 // deploy launches a new deployment unless there's already a deployment
 // process in progress for config.
 func (o DeployOptions) deploy(config *deployapi.DeploymentConfig, out io.Writer) error {
+	// TODO: This implies that deploymentconfig.status.latestVersion is always synced. Currently,
+	// that's the case because clients (oc, trigger controllers) are updating the status directly.
+	// Clients should be acting either on spec or on annotations and status updates should be a
+	// responsibility of the main controller. We need to start by unplugging this assumption from
+	// our client tools.
 	deploymentName := deployutil.LatestDeploymentNameForConfig(config)
 	deployment, err := o.kubeClient.ReplicationControllers(config.Namespace).Get(deploymentName)
 	if err == nil {
@@ -225,13 +230,17 @@ func (o DeployOptions) deploy(config *deployapi.DeploymentConfig, out io.Writer)
 		}
 	}
 
-	config.Status.LatestVersion++
-	_, err = o.osClient.DeploymentConfigs(config.Namespace).Update(config)
-	if err == nil {
-		fmt.Fprintf(out, "Started deployment #%d\n", config.Status.LatestVersion)
-		fmt.Fprintf(out, "Use '%s logs -f dc/%s' to track its progress.\n", o.baseCommandName, config.Name)
+	if config.Annotations == nil {
+		config.Annotations = make(map[string]string)
 	}
-	return err
+	config.Annotations[deployapi.DeploymentInstantiatedAnnotation] = deployapi.DeploymentInstantiatedAnnotationValue
+	dc, err := o.osClient.DeploymentConfigs(config.Namespace).Update(config)
+	if err != nil {
+		return err
+	}
+	fmt.Fprintf(out, "Started deployment #%d\n", dc.Status.LatestVersion)
+	fmt.Fprintf(out, "Use '%s logs -f dc/%s' to track its progress.\n", o.baseCommandName, dc.Name)
+	return nil
 }
 
 // retry resets the status of the latest deployment to New, which will cause
@@ -241,6 +250,11 @@ func (o DeployOptions) retry(config *deployapi.DeploymentConfig, out io.Writer)
 	if config.Status.LatestVersion == 0 {
 		return fmt.Errorf("no deployments found for %s/%s", config.Namespace, config.Name)
 	}
+	// TODO: This implies that deploymentconfig.status.latestVersion is always synced. Currently,
+	// that's the case because clients (oc, trigger controllers) are updating the status directly.
+	// Clients should be acting either on spec or on annotations and status updates should be a
+	// responsibility of the main controller. We need to start by unplugging this assumption from
+	// our client tools.
 	deploymentName := deployutil.LatestDeploymentNameForConfig(config)
 	deployment, err := o.kubeClient.ReplicationControllers(config.Namespace).Get(deploymentName)
 	if err != nil {

pkg/cmd/cli/cmd/deploy_test.go

@@ -65,9 +65,8 @@ func TestCmdDeploy_latestOk(t *testing.T) {
 		if updatedConfig == nil {
 			t.Fatalf("expected updated config")
 		}
-
-		if e, a := 2, updatedConfig.Status.LatestVersion; e != a {
-			t.Fatalf("expected updated config version %d, got %d", e, a)
+		if !deployutil.IsInstantiated(updatedConfig) {
+			t.Fatalf("expected deployment config instantiation")
 		}
 	}
 }

pkg/cmd/server/origin/master.go

@@ -374,7 +374,7 @@ func (c *MasterConfig) GetRestStorage() map[string]rest.Storage {
 	buildConfigStorage := buildconfigetcd.NewREST(c.EtcdHelper)
 	buildConfigRegistry := buildconfigregistry.NewRegistry(buildConfigStorage)
 
-	deployConfigStorage, deployConfigScaleStorage := deployconfigetcd.NewREST(c.EtcdHelper, c.DeploymentConfigScaleClient())
+	deployConfigStorage, deployConfigStatusStorage, deployConfigScaleStorage := deployconfigetcd.NewREST(c.EtcdHelper, c.DeploymentConfigScaleClient())
 	deployConfigRegistry := deployconfigregistry.NewRegistry(deployConfigStorage)
 
 	routeAllocator := c.RouteAllocator()
@@ -500,6 +500,7 @@ func (c *MasterConfig) GetRestStorage() map[string]rest.Storage {
 
 		"deploymentConfigs":         deployConfigStorage,
 		"deploymentConfigs/scale":   deployConfigScaleStorage,
+		"deploymentConfigs/status":  deployConfigStatusStorage,
 		"generateDeploymentConfigs": deployconfiggenerator.NewREST(deployConfigGenerator, c.EtcdHelper.Codec()),
 		"deploymentConfigRollbacks": deployrollback.NewREST(deployRollbackClient, c.EtcdHelper.Codec()),
 		"deploymentConfigs/log":     deploylogregistry.NewREST(configClient, kclient, c.DeploymentLogClient(), kubeletClient),

pkg/deploy/api/deep_copy_generated.go

@@ -192,6 +192,7 @@ func DeepCopy_api_DeploymentConfigStatus(in DeploymentConfigStatus, out *Deploym
 	} else {
 		out.Details = nil
 	}
+	out.ObservedGeneration = in.ObservedGeneration
 	return nil
 }
 

pkg/deploy/api/types.go

@@ -238,6 +238,9 @@ const (
 	// DeploymentReplicasAnnotation is for internal use only and is for
 	// detecting external modifications to deployment replica counts.
 	DeploymentReplicasAnnotation = "openshift.io/deployment.replicas"
+	// DeploymentInstantiatedAnnotation indicates that the deployment has been instantiated.
+	// The annotation value does not matter and its mere presence indicates instantiation.
+	DeploymentInstantiatedAnnotation = "openshift.io/deployment.instantiated"
 	// PostHookPodSuffix is the suffix added to all pre hook pods
 	PreHookPodSuffix = "hook-pre"
 	// PostHookPodSuffix is the suffix added to all mid hook pods
@@ -264,6 +267,10 @@ const MaxDeploymentDurationSeconds int64 = 21600
 // annotation that signifies that the deployment should be cancelled
 const DeploymentCancelledAnnotationValue = "true"
 
+// DeploymentInstantiatedAnnotationValue represents the value for the DeploymentInstantiatedAnnotation
+// annotation that signifies that the deployment should be instantiated.
+const DeploymentInstantiatedAnnotationValue = "true"
+
 // DeploymentConfig represents a configuration for a single deployment (represented as a
 // ReplicationController). It also contains details about changes which resulted in the current
 // state of the DeploymentConfig. Each change to the DeploymentConfig which should result in
@@ -313,6 +320,8 @@ type DeploymentConfigStatus struct {
 	// Details are the reasons for the update to this deployment config.
 	// This could be based on a change made by the user or caused by an automatic trigger
 	Details *DeploymentDetails
+	// ObservedGeneration is the most recent generation observed by the controller.
+	ObservedGeneration int64
 }
 
 // DeploymentTriggerPolicy describes a policy for a single trigger that results in a new deployment.

pkg/deploy/api/v1/conversion_generated.go

@@ -477,6 +477,7 @@ func autoConvert_v1_DeploymentConfigStatus_To_api_DeploymentConfigStatus(in *Dep
 	} else {
 		out.Details = nil
 	}
+	out.ObservedGeneration = in.ObservedGeneration
 	return nil
 }
 
@@ -498,6 +499,7 @@ func autoConvert_api_DeploymentConfigStatus_To_v1_DeploymentConfigStatus(in *dep
 	} else {
 		out.Details = nil
 	}
+	out.ObservedGeneration = in.ObservedGeneration
 	return nil
 }
 

pkg/deploy/api/v1/deep_copy_generated.go

@@ -191,6 +191,7 @@ func DeepCopy_v1_DeploymentConfigStatus(in DeploymentConfigStatus, out *Deployme
 	} else {
 		out.Details = nil
 	}
+	out.ObservedGeneration = in.ObservedGeneration
 	return nil
 }
 

pkg/deploy/api/v1/swagger_doc.go

@@ -93,9 +93,10 @@ func (DeploymentConfigSpec) SwaggerDoc() map[string]string {
 }
 
 var map_DeploymentConfigStatus = map[string]string{
-	"":              "DeploymentConfigStatus represents the current deployment state.",
-	"latestVersion": "LatestVersion is used to determine whether the current deployment associated with a DeploymentConfig is out of sync.",
-	"details":       "Details are the reasons for the update to this deployment config. This could be based on a change made by the user or caused by an automatic trigger",
+	"":                   "DeploymentConfigStatus represents the current deployment state.",
+	"latestVersion":      "LatestVersion is used to determine whether the current deployment associated with a DeploymentConfig is out of sync.",
+	"details":            "Details are the reasons for the update to this deployment config. This could be based on a change made by the user or caused by an automatic trigger",
+	"observedGeneration": "ObservedGeneration is the most recent generation observed by the controller.",
 }
 
 func (DeploymentConfigStatus) SwaggerDoc() map[string]string {

pkg/deploy/api/v1/types.go

@@ -229,6 +229,9 @@ const (
 	// DeploymentCancelledAnnotation indicates that the deployment has been cancelled
 	// The annotation value does not matter and its mere presence indicates cancellation
 	DeploymentCancelledAnnotation = "openshift.io/deployment.cancelled"
+	// DeploymentInstantiatedAnnotation indicates that the deployment has been instantiated.
+	// The annotation value does not matter and its mere presence indicates instantiation.
+	DeploymentInstantiatedAnnotation = "openshift.io/deployment.instantiated"
 )
 
 // DeploymentConfig represents a configuration for a single deployment (represented as a
@@ -281,6 +284,8 @@ type DeploymentConfigStatus struct {
 	// Details are the reasons for the update to this deployment config.
 	// This could be based on a change made by the user or caused by an automatic trigger
 	Details *DeploymentDetails `json:"details,omitempty"`
+	// ObservedGeneration is the most recent generation observed by the controller.
+	ObservedGeneration int64 `json:"observedGeneration,omitempty"`
 }
 
 // DeploymentTriggerPolicy describes a policy for a single trigger that results in a new deployment.

pkg/deploy/api/v1beta3/types.go

@@ -229,6 +229,9 @@ const (
 	// DeploymentCancelledAnnotation indicates that the deployment has been cancelled
 	// The annotation value does not matter and its mere presence indicates cancellation
 	DeploymentCancelledAnnotation = "openshift.io/deployment.cancelled"
+	// DeploymentInstantiatedAnnotation indicates that the deployment has been instantiated.
+	// The annotation value does not matter and its mere presence indicates instantiation.
+	DeploymentInstantiatedAnnotation = "openshift.io/deployment.instantiated"
 )
 
 // These constants represent the various reasons for cancelling a deployment
@@ -296,6 +299,8 @@ type DeploymentConfigStatus struct {
 	// The reasons for the update to this deployment config.
 	// This could be based on a change made by the user or caused by an automatic trigger
 	Details *DeploymentDetails `json:"details,omitempty"`
+	// ObservedGeneration is the most recent generation observed by the controller.
+	ObservedGeneration int64 `json:"observedGeneration,omitempty"`
 }
 
 // DeploymentTriggerPolicy describes a policy for a single trigger that results in a new deployment.