Skip to content

Commit

Permalink
deployapi: add generation numbers
Browse files Browse the repository at this point in the history
Add generation numbers to deploymentconfigs. Helpful for decoupling
latestVersion from oc
  • Loading branch information
0xmichalis committed May 9, 2016
1 parent 706afc8 commit f12aca3
Show file tree
Hide file tree
Showing 11 changed files with 60 additions and 11 deletions.
5 changes: 5 additions & 0 deletions api/swagger-spec/oapi-v1.json
Original file line number Diff line number Diff line change
Expand Up @@ -20531,6 +20531,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."
}
}
},
Expand Down
1 change: 1 addition & 0 deletions pkg/api/v1beta3/deep_copy_generated.go
Original file line number Diff line number Diff line change
Expand Up @@ -1611,6 +1611,7 @@ func deepCopy_v1beta3_DeploymentConfigStatus(in deployapiv1beta3.DeploymentConfi
} else {
out.Details = nil
}
out.ObservedGeneration = in.ObservedGeneration
return nil
}

Expand Down
1 change: 1 addition & 0 deletions pkg/deploy/api/deep_copy_generated.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ func DeepCopy_api_DeploymentConfigStatus(in DeploymentConfigStatus, out *Deploym
} else {
out.Details = nil
}
out.ObservedGeneration = in.ObservedGeneration
return nil
}

Expand Down
2 changes: 2 additions & 0 deletions pkg/deploy/api/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,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.
Expand Down
2 changes: 2 additions & 0 deletions pkg/deploy/api/v1/conversion_generated.go
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,7 @@ func autoConvert_v1_DeploymentConfigStatus_To_api_DeploymentConfigStatus(in *Dep
} else {
out.Details = nil
}
out.ObservedGeneration = in.ObservedGeneration
return nil
}

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

Expand Down
1 change: 1 addition & 0 deletions pkg/deploy/api/v1/deep_copy_generated.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ func DeepCopy_v1_DeploymentConfigStatus(in DeploymentConfigStatus, out *Deployme
} else {
out.Details = nil
}
out.ObservedGeneration = in.ObservedGeneration
return nil
}

Expand Down
2 changes: 2 additions & 0 deletions pkg/deploy/api/v1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,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.
Expand Down
2 changes: 2 additions & 0 deletions pkg/deploy/api/v1beta3/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,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.
Expand Down
20 changes: 14 additions & 6 deletions pkg/deploy/api/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,30 +21,35 @@ func ValidateDeploymentConfig(config *deployapi.DeploymentConfig) field.ErrorLis
allErrs := validation.ValidateObjectMeta(&config.ObjectMeta, true, validation.NameIsDNSSubdomain, field.NewPath("metadata"))

// TODO: Refactor to validate spec and status separately
specPath := field.NewPath("spec")
for i := range config.Spec.Triggers {
allErrs = append(allErrs, validateTrigger(&config.Spec.Triggers[i], field.NewPath("spec", "triggers").Index(i))...)
allErrs = append(allErrs, validateTrigger(&config.Spec.Triggers[i], specPath.Child("triggers").Index(i))...)
}

var spec *kapi.PodSpec
if config.Spec.Template != nil {
spec = &config.Spec.Template.Spec
}
specPath := field.NewPath("spec")
allErrs = append(allErrs, validateDeploymentStrategy(&config.Spec.Strategy, spec, field.NewPath("spec", "strategy"))...)
allErrs = append(allErrs, validateDeploymentStrategy(&config.Spec.Strategy, spec, specPath.Child("strategy"))...)
if config.Spec.Template == nil {
allErrs = append(allErrs, field.Required(specPath.Child("template"), ""))
} else {
allErrs = append(allErrs, validation.ValidatePodTemplateSpec(config.Spec.Template, specPath.Child("template"))...)
}
if config.Status.LatestVersion < 0 {
allErrs = append(allErrs, field.Invalid(field.NewPath("status", "latestVersion"), config.Status.LatestVersion, "latestVersion cannot be negative"))
}
if config.Spec.Replicas < 0 {
allErrs = append(allErrs, field.Invalid(specPath.Child("replicas"), config.Spec.Replicas, "replicas cannot be negative"))
}
if len(config.Spec.Selector) == 0 {
allErrs = append(allErrs, field.Invalid(specPath.Child("selector"), config.Spec.Selector, "selector cannot be empty"))
}

statusPath := field.NewPath("status")
if config.Status.LatestVersion < 0 {
allErrs = append(allErrs, field.Invalid(statusPath.Child("latestVersion"), config.Status.LatestVersion, "latestVersion cannot be negative"))
}
if config.Status.ObservedGeneration < int64(0) {
allErrs = append(allErrs, field.Invalid(statusPath.Child("observedGeneration"), config.Status.ObservedGeneration, "observedGeneration cannot be negative"))
}
return allErrs
}

Expand All @@ -57,6 +62,9 @@ func ValidateDeploymentConfigUpdate(newConfig *deployapi.DeploymentConfig, oldCo
} else if newConfig.Status.LatestVersion > (oldConfig.Status.LatestVersion + 1) {
allErrs = append(allErrs, field.Invalid(statusPath.Child("latestVersion"), newConfig.Status.LatestVersion, "latestVersion can only be incremented by 1"))
}
if newConfig.Status.ObservedGeneration < oldConfig.Status.ObservedGeneration {
allErrs = append(allErrs, field.Invalid(statusPath.Child("observedGeneration"), newConfig.Status.ObservedGeneration, "observedGeneration cannot be decremented"))
}
return allErrs
}

Expand Down
13 changes: 10 additions & 3 deletions pkg/deploy/controller/deploymentconfig/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,10 @@ func (c *DeploymentConfigController) Handle(config *deployapi.DeploymentConfig)
return fmt.Errorf("couldn't create deployment for deployment config %s: %v", deployutil.LabelForDeploymentConfig(config), err)
}
c.recorder.Eventf(config, kapi.EventTypeNormal, "DeploymentCreated", "Created new deployment %q for version %d", created.Name, config.Status.LatestVersion)
return nil
// Supposedly we have to update the dc status here; will get the separate status call with https://github.com/openshift/origin/pull/6479
config.Status.ObservedGeneration = int(config.Generation)
_, err = c.osClient.DeploymentConfigs(config.Namespace).Update(config)
return err
}

// reconcileDeployments reconciles existing deployment replica counts which
Expand Down Expand Up @@ -219,7 +222,8 @@ func (c *DeploymentConfigController) reconcileDeployments(existingDeployments *k
default:
oldReplicas := config.Spec.Replicas
config.Spec.Replicas = activeReplicas
_, err := c.osClient.DeploymentConfigs(config.Namespace).Update(config)
var err error
config, err = c.osClient.DeploymentConfigs(config.Namespace).Update(config)
if err != nil {
return err
}
Expand Down Expand Up @@ -260,5 +264,8 @@ func (c *DeploymentConfigController) reconcileDeployments(existingDeployments *k
}
}
}
return nil
// Supposedly we have to update the dc status here; will get the separate status call with https://github.com/openshift/origin/pull/6479
config.Status.ObservedGeneration = int(config.Generation)
_, err := c.osClient.DeploymentConfigs(config.Namespace).Update(config)
return err
}
22 changes: 20 additions & 2 deletions pkg/deploy/registry/deployconfig/strategy.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package deployconfig

import (
"fmt"
"reflect"

kapi "k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/fields"
Expand Down Expand Up @@ -39,14 +40,31 @@ func (strategy) AllowUnconditionalUpdate() bool {

// PrepareForCreate clears fields that are not allowed to be set by end users on creation.
func (strategy) PrepareForCreate(obj runtime.Object) {
_ = obj.(*api.DeploymentConfig)
dc := obj.(*api.DeploymentConfig)
// TODO: need to ensure status.latestVersion is not set out of order
dc.Generation = 1
}

// PrepareForUpdate clears fields that are not allowed to be set by end users on update.
func (strategy) PrepareForUpdate(obj, old runtime.Object) {
_ = obj.(*api.DeploymentConfig)
oldDc := obj.(*api.DeploymentConfig)
newDc := old.(*api.DeploymentConfig)
// TODO: need to ensure status.latestVersion is not set out of order

// Any changes to the spec increment the generation number, any changes to the
// status should reflect the generation number of the corresponding object. We push
// the burden of managing the status onto the clients because we can't (in general)
// know here what version of spec the writer of the status has seen. It may seem like
// we can at first -- since obj contains spec -- but in the future we will probably make
// status its own object, and even if we don't, writes may be the result of a
// read-update-write loop, so the contents of spec may not actually be the spec that
// the controller has *seen*.
//
// TODO: Any changes to a part of the object that represents desired state (labels,
// annotations etc) should also increment the generation.
if !reflect.DeepEqual(oldDc.Spec, newDc.Spec) {
newDc.Generation = oldDc.Generation + 1
}
}

// Canonicalize normalizes the object after validation.
Expand Down

0 comments on commit f12aca3

Please sign in to comment.