diff --git a/api/v1alpha4/machinedeployment_types.go b/api/v1alpha4/machinedeployment_types.go index b3eae32d8d0b..1882094c5ce8 100644 --- a/api/v1alpha4/machinedeployment_types.go +++ b/api/v1alpha4/machinedeployment_types.go @@ -53,6 +53,10 @@ const ( // is machinedeployment.spec.replicas + maxSurge. Used by the underlying machine sets to estimate their // proportions in case the deployment has surge replicas. MaxReplicasAnnotation = "machinedeployment.clusters.x-k8s.io/max-replicas" + + // MachineDeploymentUniqueLabel is the label applied to Machines + // in a MachineDeployment containing the hash of the template. + MachineDeploymentUniqueLabel = "machine-template-hash" ) // ANCHOR: MachineDeploymentSpec diff --git a/api/v1beta1/machinedeployment_types.go b/api/v1beta1/machinedeployment_types.go index f43d2d4df5fc..a450a37bea37 100644 --- a/api/v1beta1/machinedeployment_types.go +++ b/api/v1beta1/machinedeployment_types.go @@ -53,6 +53,10 @@ const ( // is machinedeployment.spec.replicas + maxSurge. Used by the underlying machine sets to estimate their // proportions in case the deployment has surge replicas. MaxReplicasAnnotation = "machinedeployment.clusters.x-k8s.io/max-replicas" + + // MachineDeploymentUniqueLabel is the label applied to Machines + // in a MachineDeployment containing the hash of the template. + MachineDeploymentUniqueLabel = "machine-template-hash" ) // ANCHOR: MachineDeploymentSpec diff --git a/cmd/clusterctl/client/alpha/machinedeployment.go b/cmd/clusterctl/client/alpha/machinedeployment.go index 15d6ad98a7b3..11104249330e 100644 --- a/cmd/clusterctl/client/alpha/machinedeployment.go +++ b/cmd/clusterctl/client/alpha/machinedeployment.go @@ -17,13 +17,16 @@ limitations under the License. package alpha import ( + "strconv" + "github.com/pkg/errors" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/runtime" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" "sigs.k8s.io/cluster-api/cmd/clusterctl/client/cluster" logf "sigs.k8s.io/cluster-api/cmd/clusterctl/log" - "sigs.k8s.io/cluster-api/controllers/mdutil" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -75,7 +78,7 @@ func findMachineDeploymentRevision(toRevision int64, allMSs []*clusterv1.Machine previousRevision = int64(-1) ) for _, ms := range allMSs { - if v, err := mdutil.Revision(ms); err == nil { + if v, err := revision(ms); err == nil { if toRevision == 0 { if latestRevision < v { // newest one we've seen so far @@ -147,3 +150,15 @@ func getMachineSetsForDeployment(proxy cluster.Proxy, d *clusterv1.MachineDeploy return filtered, nil } + +func revision(obj runtime.Object) (int64, error) { + acc, err := meta.Accessor(obj) + if err != nil { + return 0, err + } + v, ok := acc.GetAnnotations()[clusterv1.RevisionAnnotation] + if !ok { + return 0, nil + } + return strconv.ParseInt(v, 10, 64) +} diff --git a/cmd/clusterctl/client/alpha/rollout_rollbacker.go b/cmd/clusterctl/client/alpha/rollout_rollbacker.go index 09fb382df545..0ad6dccc8079 100644 --- a/cmd/clusterctl/client/alpha/rollout_rollbacker.go +++ b/cmd/clusterctl/client/alpha/rollout_rollbacker.go @@ -22,7 +22,6 @@ import ( clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" "sigs.k8s.io/cluster-api/cmd/clusterctl/client/cluster" logf "sigs.k8s.io/cluster-api/cmd/clusterctl/log" - "sigs.k8s.io/cluster-api/controllers/mdutil" "sigs.k8s.io/cluster-api/util/patch" ) @@ -73,7 +72,7 @@ func rollbackMachineDeployment(proxy cluster.Proxy, d *clusterv1.MachineDeployme } // Copy template into the machinedeployment (excluding the hash) revMSTemplate := *msForRevision.Spec.Template.DeepCopy() - delete(revMSTemplate.Labels, mdutil.DefaultMachineDeploymentUniqueLabelKey) + delete(revMSTemplate.Labels, clusterv1.MachineDeploymentUniqueLabel) d.Spec.Template = revMSTemplate return patchHelper.Patch(ctx, d) diff --git a/controllers/mdutil/doc.go b/controllers/internal/mdutil/doc.go similarity index 84% rename from controllers/mdutil/doc.go rename to controllers/internal/mdutil/doc.go index 6653a2265108..b2dedac92014 100644 --- a/controllers/mdutil/doc.go +++ b/controllers/internal/mdutil/doc.go @@ -14,5 +14,6 @@ See the License for the specific language governing permissions and limitations under the License. */ -// Package mdutil implements MachineDeployment utilities. +// Package mdutil implements MachineDeployment utilities +// meant to be consumed internally by the controller. package mdutil diff --git a/controllers/mdutil/util.go b/controllers/internal/mdutil/util.go similarity index 90% rename from controllers/mdutil/util.go rename to controllers/internal/mdutil/util.go index c9358e778bfc..05f710fbbe27 100644 --- a/controllers/mdutil/util.go +++ b/controllers/internal/mdutil/util.go @@ -38,33 +38,6 @@ import ( "sigs.k8s.io/cluster-api/util/conversion" ) -const ( - // DefaultMachineDeploymentUniqueLabelKey is the label applied to Machines - // in a MachineDeployment containing the hash of the template. - DefaultMachineDeploymentUniqueLabelKey = "machine-template-hash" - - // FailedMSCreateReason is added in a machine deployment when it cannot create a new machine set. - // Deprecated: This field will be removed in a next release. - FailedMSCreateReason = "MachineSetCreateError" - - // FoundNewMSReason is added in a machine deployment when it adopts an existing machine set. - // Deprecated: This field will be removed in a next release. - FoundNewMSReason = "FoundNewMachineSet" - - // PausedDeployReason is added in a deployment when it is paused. Lack of progress shouldn't be - // estimated once a deployment is paused. - // Deprecated: This field will be removed in a next release. - PausedDeployReason = "DeploymentPaused" - - // MinimumReplicasAvailable is added in a deployment when it has its minimum replicas required available. - // Deprecated: This field will be removed in a next release. - MinimumReplicasAvailable = "MinimumReplicasAvailable" - // MinimumReplicasUnavailable is added in a deployment when it doesn't have the minimum required replicas - // available. - // Deprecated: This field will be removed in a next release. - MinimumReplicasUnavailable = "MinimumReplicasUnavailable" -) - // MachineSetsByCreationTimestamp sorts a list of MachineSet by creation timestamp, using their names as a tie breaker. type MachineSetsByCreationTimestamp []*clusterv1.MachineSet @@ -299,9 +272,8 @@ func SetReplicasAnnotations(ms *clusterv1.MachineSet, desiredReplicas, maxReplic ms.Annotations[clusterv1.DesiredReplicasAnnotation] = desiredString updated = true } - maxString := fmt.Sprintf("%d", maxReplicas) - if hasString := ms.Annotations[clusterv1.MaxReplicasAnnotation]; hasString != maxString { - ms.Annotations[clusterv1.MaxReplicasAnnotation] = maxString + if hasString := ms.Annotations[clusterv1.MaxReplicasAnnotation]; hasString != fmt.Sprintf("%d", maxReplicas) { + ms.Annotations[clusterv1.MaxReplicasAnnotation] = fmt.Sprintf("%d", maxReplicas) updated = true } return updated @@ -316,8 +288,7 @@ func ReplicasAnnotationsNeedUpdate(ms *clusterv1.MachineSet, desiredReplicas, ma if hasString := ms.Annotations[clusterv1.DesiredReplicasAnnotation]; hasString != desiredString { return true } - maxString := fmt.Sprintf("%d", maxReplicas) - if hasString := ms.Annotations[clusterv1.MaxReplicasAnnotation]; hasString != maxString { + if hasString := ms.Annotations[clusterv1.MaxReplicasAnnotation]; hasString != fmt.Sprintf("%d", maxReplicas) { return true } return false @@ -403,8 +374,8 @@ func EqualMachineTemplate(template1, template2 *clusterv1.MachineTemplateSpec) b // 1. The hash result would be different upon machineTemplateSpec API changes // (e.g. the addition of a new field will cause the hash code to change) // 2. The deployment template won't have hash labels - delete(t1Copy.Labels, DefaultMachineDeploymentUniqueLabelKey) - delete(t2Copy.Labels, DefaultMachineDeploymentUniqueLabelKey) + delete(t1Copy.Labels, clusterv1.MachineDeploymentUniqueLabel) + delete(t2Copy.Labels, clusterv1.MachineDeploymentUniqueLabel) // Remove the version part from the references APIVersion field, // for more details see issue #2183 and #2140. @@ -697,23 +668,6 @@ func CloneSelectorAndAddLabel(selector *metav1.LabelSelector, labelKey, labelVal return newSelector } -// DeepHashObject writes specified object to hash using the spew library -// which follows pointers and prints actual values of the nested objects -// ensuring the hash does not change when a pointer changes. -// Deprecated: Please use controllers/mdutil SpewHashObject(hasher, objectToWrite). -func DeepHashObject(hasher hash.Hash, objectToWrite interface{}) { - hasher.Reset() - printer := spew.ConfigState{ - Indent: " ", - SortKeys: true, - DisableMethods: true, - SpewKeys: true, - } - // We ignore the returned error because there is no way to return the error without - // breaking API compatibility. Please use SpewHashObject instead. - _, _ = printer.Fprintf(hasher, "%#v", objectToWrite) -} - // SpewHashObject writes specified object to hash using the spew library // which follows pointers and prints actual values of the nested objects // ensuring the hash does not change when a pointer changes. @@ -732,18 +686,10 @@ func SpewHashObject(hasher hash.Hash, objectToWrite interface{}) error { return nil } -// ComputeHash computes the hash of a MachineTemplateSpec using the spew library. -// Deprecated: Please use controllers/mdutil ComputeSpewHash(template). -func ComputeHash(template *clusterv1.MachineTemplateSpec) uint32 { - machineTemplateSpecHasher := fnv.New32a() - DeepHashObject(machineTemplateSpecHasher, *template) - return machineTemplateSpecHasher.Sum32() -} - // ComputeSpewHash computes the hash of a MachineTemplateSpec using the spew library. -func ComputeSpewHash(template *clusterv1.MachineTemplateSpec) (uint32, error) { +func ComputeSpewHash(objectToWrite interface{}) (uint32, error) { machineTemplateSpecHasher := fnv.New32a() - if err := SpewHashObject(machineTemplateSpecHasher, *template); err != nil { + if err := SpewHashObject(machineTemplateSpecHasher, objectToWrite); err != nil { return 0, err } return machineTemplateSpecHasher.Sum32(), nil diff --git a/controllers/mdutil/util_test.go b/controllers/internal/mdutil/util_test.go similarity index 95% rename from controllers/mdutil/util_test.go rename to controllers/internal/mdutil/util_test.go index ece993de2fb8..8838724f3b8e 100644 --- a/controllers/mdutil/util_test.go +++ b/controllers/internal/mdutil/util_test.go @@ -24,7 +24,6 @@ import ( "time" . "github.com/onsi/gomega" - corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -106,62 +105,62 @@ func TestEqualMachineTemplate(t *testing.T) { }{ { Name: "Same spec, same labels", - Former: generateMachineTemplateSpec(map[string]string{}, map[string]string{DefaultMachineDeploymentUniqueLabelKey: "value-1", "something": "else"}), - Latter: generateMachineTemplateSpec(map[string]string{}, map[string]string{DefaultMachineDeploymentUniqueLabelKey: "value-1", "something": "else"}), + Former: generateMachineTemplateSpec(map[string]string{}, map[string]string{clusterv1.MachineDeploymentUniqueLabel: "value-1", "something": "else"}), + Latter: generateMachineTemplateSpec(map[string]string{}, map[string]string{clusterv1.MachineDeploymentUniqueLabel: "value-1", "something": "else"}), Expected: true, }, { Name: "Same spec, only machine-template-hash label value is different", - Former: generateMachineTemplateSpec(map[string]string{}, map[string]string{DefaultMachineDeploymentUniqueLabelKey: "value-1", "something": "else"}), - Latter: generateMachineTemplateSpec(map[string]string{}, map[string]string{DefaultMachineDeploymentUniqueLabelKey: "value-2", "something": "else"}), + Former: generateMachineTemplateSpec(map[string]string{}, map[string]string{clusterv1.MachineDeploymentUniqueLabel: "value-1", "something": "else"}), + Latter: generateMachineTemplateSpec(map[string]string{}, map[string]string{clusterv1.MachineDeploymentUniqueLabel: "value-2", "something": "else"}), Expected: true, }, { Name: "Same spec, the former doesn't have machine-template-hash label", Former: generateMachineTemplateSpec(map[string]string{}, map[string]string{"something": "else"}), - Latter: generateMachineTemplateSpec(map[string]string{}, map[string]string{DefaultMachineDeploymentUniqueLabelKey: "value-2", "something": "else"}), + Latter: generateMachineTemplateSpec(map[string]string{}, map[string]string{clusterv1.MachineDeploymentUniqueLabel: "value-2", "something": "else"}), Expected: true, }, { Name: "Same spec, the former doesn't have machine-template-hash label", Former: generateMachineTemplateSpec(map[string]string{}, map[string]string{"something": "else"}), - Latter: generateMachineTemplateSpec(map[string]string{}, map[string]string{DefaultMachineDeploymentUniqueLabelKey: "value-2", "something": "else"}), + Latter: generateMachineTemplateSpec(map[string]string{}, map[string]string{clusterv1.MachineDeploymentUniqueLabel: "value-2", "something": "else"}), Expected: true, }, { Name: "Same spec, the label is different, the former doesn't have machine-template-hash label, same number of labels", Former: generateMachineTemplateSpec(map[string]string{}, map[string]string{"something": "else"}), - Latter: generateMachineTemplateSpec(map[string]string{}, map[string]string{DefaultMachineDeploymentUniqueLabelKey: "value-2"}), + Latter: generateMachineTemplateSpec(map[string]string{}, map[string]string{clusterv1.MachineDeploymentUniqueLabel: "value-2"}), Expected: false, }, { Name: "Same spec, the label is different, the latter doesn't have machine-template-hash label, same number of labels", - Former: generateMachineTemplateSpec(map[string]string{}, map[string]string{DefaultMachineDeploymentUniqueLabelKey: "value-1"}), + Former: generateMachineTemplateSpec(map[string]string{}, map[string]string{clusterv1.MachineDeploymentUniqueLabel: "value-1"}), Latter: generateMachineTemplateSpec(map[string]string{}, map[string]string{"something": "else"}), Expected: false, }, { Name: "Same spec, the label is different, and the machine-template-hash label value is the same", - Former: generateMachineTemplateSpec(map[string]string{}, map[string]string{DefaultMachineDeploymentUniqueLabelKey: "value-1"}), - Latter: generateMachineTemplateSpec(map[string]string{}, map[string]string{DefaultMachineDeploymentUniqueLabelKey: "value-1", "something": "else"}), + Former: generateMachineTemplateSpec(map[string]string{}, map[string]string{clusterv1.MachineDeploymentUniqueLabel: "value-1"}), + Latter: generateMachineTemplateSpec(map[string]string{}, map[string]string{clusterv1.MachineDeploymentUniqueLabel: "value-1", "something": "else"}), Expected: false, }, { Name: "Different spec, same labels", - Former: generateMachineTemplateSpec(map[string]string{"former": "value"}, map[string]string{DefaultMachineDeploymentUniqueLabelKey: "value-1", "something": "else"}), - Latter: generateMachineTemplateSpec(map[string]string{"latter": "value"}, map[string]string{DefaultMachineDeploymentUniqueLabelKey: "value-1", "something": "else"}), + Former: generateMachineTemplateSpec(map[string]string{"former": "value"}, map[string]string{clusterv1.MachineDeploymentUniqueLabel: "value-1", "something": "else"}), + Latter: generateMachineTemplateSpec(map[string]string{"latter": "value"}, map[string]string{clusterv1.MachineDeploymentUniqueLabel: "value-1", "something": "else"}), Expected: false, }, { Name: "Different spec, different machine-template-hash label value", - Former: generateMachineTemplateSpec(map[string]string{"x": ""}, map[string]string{DefaultMachineDeploymentUniqueLabelKey: "value-1", "something": "else"}), - Latter: generateMachineTemplateSpec(map[string]string{"x": "1"}, map[string]string{DefaultMachineDeploymentUniqueLabelKey: "value-2", "something": "else"}), + Former: generateMachineTemplateSpec(map[string]string{"x": ""}, map[string]string{clusterv1.MachineDeploymentUniqueLabel: "value-1", "something": "else"}), + Latter: generateMachineTemplateSpec(map[string]string{"x": "1"}, map[string]string{clusterv1.MachineDeploymentUniqueLabel: "value-2", "something": "else"}), Expected: false, }, { Name: "Different spec, the former doesn't have machine-template-hash label", Former: generateMachineTemplateSpec(map[string]string{"x": ""}, map[string]string{"something": "else"}), - Latter: generateMachineTemplateSpec(map[string]string{"x": "1"}, map[string]string{DefaultMachineDeploymentUniqueLabelKey: "value-2", "something": "else"}), + Latter: generateMachineTemplateSpec(map[string]string{"x": "1"}, map[string]string{clusterv1.MachineDeploymentUniqueLabel: "value-2", "something": "else"}), Expected: false, }, { @@ -273,11 +272,11 @@ func TestFindNewMachineSet(t *testing.T) { deployment := generateDeployment("nginx") newMS := generateMS(deployment) - newMS.Labels[DefaultMachineDeploymentUniqueLabelKey] = "hash" + newMS.Labels[clusterv1.MachineDeploymentUniqueLabel] = "hash" newMS.CreationTimestamp = later newMSDup := generateMS(deployment) - newMSDup.Labels[DefaultMachineDeploymentUniqueLabelKey] = "different-hash" + newMSDup.Labels[clusterv1.MachineDeploymentUniqueLabel] = "different-hash" newMSDup.CreationTimestamp = now oldDeployment := generateDeployment("nginx") @@ -331,11 +330,11 @@ func TestFindOldMachineSets(t *testing.T) { deployment := generateDeployment("nginx") newMS := generateMS(deployment) *(newMS.Spec.Replicas) = 1 - newMS.Labels[DefaultMachineDeploymentUniqueLabelKey] = "hash" + newMS.Labels[clusterv1.MachineDeploymentUniqueLabel] = "hash" newMS.CreationTimestamp = later newMSDup := generateMS(deployment) - newMSDup.Labels[DefaultMachineDeploymentUniqueLabelKey] = "different-hash" + newMSDup.Labels[clusterv1.MachineDeploymentUniqueLabel] = "different-hash" newMSDup.CreationTimestamp = now oldDeployment := generateDeployment("nginx") diff --git a/controllers/machinedeployment_rolling.go b/controllers/machinedeployment_rolling.go index 726a47957b43..6e644d61fafc 100644 --- a/controllers/machinedeployment_rolling.go +++ b/controllers/machinedeployment_rolling.go @@ -23,7 +23,7 @@ import ( "github.com/pkg/errors" "k8s.io/utils/integer" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" - "sigs.k8s.io/cluster-api/controllers/mdutil" + "sigs.k8s.io/cluster-api/controllers/internal/mdutil" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" ) diff --git a/controllers/machinedeployment_rolling_test.go b/controllers/machinedeployment_rolling_test.go index 0425d56978fc..4de68a746124 100644 --- a/controllers/machinedeployment_rolling_test.go +++ b/controllers/machinedeployment_rolling_test.go @@ -26,7 +26,7 @@ import ( "k8s.io/client-go/tools/record" "k8s.io/utils/pointer" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" - "sigs.k8s.io/cluster-api/controllers/mdutil" + "sigs.k8s.io/cluster-api/controllers/internal/mdutil" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" ) diff --git a/controllers/machinedeployment_rollout_ondelete.go b/controllers/machinedeployment_rollout_ondelete.go index 4cdfd75ee0dd..09f28816a557 100644 --- a/controllers/machinedeployment_rollout_ondelete.go +++ b/controllers/machinedeployment_rollout_ondelete.go @@ -23,7 +23,7 @@ import ( "github.com/pkg/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" - "sigs.k8s.io/cluster-api/controllers/mdutil" + "sigs.k8s.io/cluster-api/controllers/internal/mdutil" "sigs.k8s.io/cluster-api/util/patch" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" diff --git a/controllers/machinedeployment_sync.go b/controllers/machinedeployment_sync.go index 032a6f4f49bc..9d3780a7946e 100644 --- a/controllers/machinedeployment_sync.go +++ b/controllers/machinedeployment_sync.go @@ -30,7 +30,7 @@ import ( "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/util/retry" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" - "sigs.k8s.io/cluster-api/controllers/mdutil" + "sigs.k8s.io/cluster-api/controllers/internal/mdutil" "sigs.k8s.io/cluster-api/feature" "sigs.k8s.io/cluster-api/util" "sigs.k8s.io/cluster-api/util/conditions" @@ -147,11 +147,11 @@ func (r *MachineDeploymentReconciler) getNewMachineSet(ctx context.Context, d *c } machineTemplateSpecHash := fmt.Sprintf("%d", hash) newMSTemplate.Labels = mdutil.CloneAndAddLabel(d.Spec.Template.Labels, - mdutil.DefaultMachineDeploymentUniqueLabelKey, machineTemplateSpecHash) + clusterv1.MachineDeploymentUniqueLabel, machineTemplateSpecHash) // Add machineTemplateHash label to selector. newMSSelector := mdutil.CloneSelectorAndAddLabel(&d.Spec.Selector, - mdutil.DefaultMachineDeploymentUniqueLabelKey, machineTemplateSpecHash) + clusterv1.MachineDeploymentUniqueLabel, machineTemplateSpecHash) minReadySeconds := int32(0) if d.Spec.MinReadySeconds != nil { diff --git a/controllers/machinedeployment_sync_test.go b/controllers/machinedeployment_sync_test.go index 73366f8ecb61..994426d89fb2 100644 --- a/controllers/machinedeployment_sync_test.go +++ b/controllers/machinedeployment_sync_test.go @@ -28,7 +28,7 @@ import ( "k8s.io/client-go/tools/record" "k8s.io/utils/pointer" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" - "sigs.k8s.io/cluster-api/controllers/mdutil" + "sigs.k8s.io/cluster-api/controllers/internal/mdutil" capierrors "sigs.k8s.io/cluster-api/errors" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" diff --git a/controllers/topology/internal/scope/state.go b/controllers/topology/internal/scope/state.go index da9d72b4962e..f4dbe6c2a97b 100644 --- a/controllers/topology/internal/scope/state.go +++ b/controllers/topology/internal/scope/state.go @@ -19,7 +19,7 @@ package scope import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" - "sigs.k8s.io/cluster-api/controllers/mdutil" + "sigs.k8s.io/cluster-api/controllers/internal/mdutil" ) // ClusterState holds all the objects representing the state of a managed Cluster topology.