Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for MachineDeployment to detect infrastructure spec changes #1114

Closed
wants to merge 1 commit into from
Closed

Add support for MachineDeployment to detect infrastructure spec changes #1114

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions pkg/controller/machinedeployment/sync.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package machinedeployment

import (
"context"
"fmt"
"reflect"
"sort"
"strconv"
Expand Down Expand Up @@ -72,7 +71,7 @@ func (r *ReconcileMachineDeployment) sync(d *clusterv1.MachineDeployment, msList
// Note that currently the deployment controller is using caches to avoid querying the server for reads.
// This may lead to stale reads of machine sets, thus incorrect deployment status.
func (r *ReconcileMachineDeployment) getAllMachineSetsAndSyncRevision(d *clusterv1.MachineDeployment, msList []*clusterv1.MachineSet, machineMap map[types.UID]*clusterv1.MachineList, createIfNotExisted bool) (*clusterv1.MachineSet, []*clusterv1.MachineSet, error) {
_, allOldMSs := dutil.FindOldMachineSets(d, msList)
_, allOldMSs := dutil.FindOldMachineSets(r.Client, d, msList)

// Get new machine set with the updated revision number
newMS, err := r.getNewMachineSet(d, msList, allOldMSs, createIfNotExisted)
Expand All @@ -89,7 +88,7 @@ func (r *ReconcileMachineDeployment) getAllMachineSetsAndSyncRevision(d *cluster
// 3. If there's no existing new MS and createIfNotExisted is true, create one with appropriate revision number (maxOldRevision + 1) and replicas.
// Note that the machine-template-hash will be added to adopted MSes and machines.
func (r *ReconcileMachineDeployment) getNewMachineSet(d *clusterv1.MachineDeployment, msList, oldMSs []*clusterv1.MachineSet, createIfNotExisted bool) (*clusterv1.MachineSet, error) {
existingNewMS := dutil.FindNewMachineSet(d, msList)
existingNewMS := dutil.FindNewMachineSet(r.Client, d, msList)

// Calculate the max revision number among all old MSes
maxOldRevision := dutil.MaxRevision(oldMSs)
Expand Down Expand Up @@ -124,9 +123,13 @@ func (r *ReconcileMachineDeployment) getNewMachineSet(d *clusterv1.MachineDeploy
return nil, nil
}

// new MachineSet does not exist, create one.
// New MachineSet does not exist, create one.
newMSTemplate := *d.Spec.Template.DeepCopy()
machineTemplateSpecHash := fmt.Sprintf("%d", dutil.ComputeHash(&newMSTemplate))
machineTemplateSpecHash, err := dutil.ComputeHash(r.Client, d.Namespace, &newMSTemplate)
if err != nil {
return nil, err
}

newMSTemplate.Labels = dutil.CloneAndAddLabel(d.Spec.Template.Labels,
dutil.DefaultMachineDeploymentUniqueLabelKey, machineTemplateSpecHash)

Expand Down
60 changes: 51 additions & 9 deletions pkg/controller/machinedeployment/util/util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,20 @@ import (
"strings"

"github.com/davecgh/go-spew/spew"
"github.com/pkg/errors"
v1 "k8s.io/api/core/v1"
apiequality "k8s.io/apimachinery/pkg/api/equality"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
intstrutil "k8s.io/apimachinery/pkg/util/intstr"
"k8s.io/klog"
"k8s.io/utils/integer"
"sigs.k8s.io/cluster-api/pkg/apis/cluster/common"
"sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha2"
"sigs.k8s.io/cluster-api/pkg/controller/external"
"sigs.k8s.io/controller-runtime/pkg/client"
)

const (
Expand Down Expand Up @@ -406,16 +410,41 @@ func EqualIgnoreHash(template1, template2 *v1alpha2.MachineTemplateSpec) bool {
return apiequality.Semantic.DeepEqual(t1Copy, t2Copy)
}

func Equals(c client.Client, namespace string, template1, template2 *v1alpha2.MachineTemplateSpec) (bool, error) {
t1Copy := template1.DeepCopy()
t2Copy := template2.DeepCopy()
delete(t1Copy.Labels, DefaultMachineDeploymentUniqueLabelKey)
delete(t2Copy.Labels, DefaultMachineDeploymentUniqueLabelKey)
h1, err := ComputeHash(c, namespace, t1Copy)
if err != nil {
return false, err
}
h2, err := ComputeHash(c, namespace, t2Copy)
if err != nil {
return false, err
}
return h1 != "" && h1 == h2, nil
}

// FindNewMachineSet returns the new MS this given deployment targets (the one with the same machine template).
func FindNewMachineSet(deployment *v1alpha2.MachineDeployment, msList []*v1alpha2.MachineSet) *v1alpha2.MachineSet {
func FindNewMachineSet(c client.Client, d *v1alpha2.MachineDeployment, msList []*v1alpha2.MachineSet) *v1alpha2.MachineSet {
sort.Sort(MachineSetsByCreationTimestamp(msList))
for i := range msList {
if EqualIgnoreHash(&msList[i].Spec.Template, &deployment.Spec.Template) {

if ok, err := Equals(c, d.Namespace, &msList[i].Spec.Template, &d.Spec.Template); ok {
// In rare cases, such as after cluster upgrades, Deployment may end up with
// having more than one new MachineSets that have the same template,
// see https://github.com/kubernetes/kubernetes/issues/40415
// We deterministically choose the oldest new MachineSet with matching template hash.
return msList[i]
} else if err != nil && EqualIgnoreHash(&msList[i].Spec.Template, &d.Spec.Template) {
// Fallback to old equal if there is an error.
klog.Warningf("Fallback hash occurred for Machine Deployment %q in namespace %q, got error: %v",
d.Name, d.Namespace, err)
return msList[i]
} else if err != nil {
klog.Errorf("Error calculating hash for Machine Deployment %q in namespace %q, got error: %v",
d.Name, d.Namespace, err)
}
}
// new MachineSet does not exist.
Expand All @@ -426,10 +455,10 @@ func FindNewMachineSet(deployment *v1alpha2.MachineDeployment, msList []*v1alpha
// Returns two list of machine sets
// - the first contains all old machine sets with all non-zero replicas
// - the second contains all old machine sets
func FindOldMachineSets(deployment *v1alpha2.MachineDeployment, msList []*v1alpha2.MachineSet) ([]*v1alpha2.MachineSet, []*v1alpha2.MachineSet) {
func FindOldMachineSets(c client.Client, deployment *v1alpha2.MachineDeployment, msList []*v1alpha2.MachineSet) ([]*v1alpha2.MachineSet, []*v1alpha2.MachineSet) {
var requiredMSs []*v1alpha2.MachineSet
allMSs := make([]*v1alpha2.MachineSet, 0, len(msList))
newMS := FindNewMachineSet(deployment, msList)
newMS := FindNewMachineSet(c, deployment, msList)
for _, ms := range msList {
// Filter out new machine set
if newMS != nil && ms.UID == newMS.UID {
Expand Down Expand Up @@ -676,7 +705,6 @@ func CloneSelectorAndAddLabel(selector *metav1.LabelSelector, labelKey, labelVal
// which follows pointers and prints actual values of the nested objects
// ensuring the hash does not change when a pointer changes.
func DeepHashObject(hasher hash.Hash, objectToWrite interface{}) {
hasher.Reset()
printer := spew.ConfigState{
Indent: " ",
SortKeys: true,
Expand All @@ -686,9 +714,23 @@ func DeepHashObject(hasher hash.Hash, objectToWrite interface{}) {
printer.Fprintf(hasher, "%#v", objectToWrite)
}

func ComputeHash(template *v1alpha2.MachineTemplateSpec) uint32 {
machineTemplateSpecHasher := fnv.New32a()
DeepHashObject(machineTemplateSpecHasher, *template)
func ComputeHash(c client.Client, namespace string, template *v1alpha2.MachineTemplateSpec) (string, error) {
hasher := fnv.New32a()

// Compute hash of the template.
DeepHashObject(hasher, template)

// Get and compute hash of infrastructure config.
infraConfig, err := external.Get(c, &template.Spec.InfrastructureRef, namespace)
if err != nil {
return "", err
}
infraSpec, _, err := unstructured.NestedMap(infraConfig.Object, "spec")
if err != nil {
return "", errors.Wrap(err, "failed to retrieve spec from infrastructure provider object")
}
DeepHashObject(hasher, infraSpec)

return machineTemplateSpecHasher.Sum32()
// Return the hash.
return fmt.Sprintf("%d", hasher.Sum32()), nil
}
54 changes: 50 additions & 4 deletions pkg/controller/machinedeployment/util/util_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,39 @@ import (
"time"

"github.com/pkg/errors"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/intstr"
"k8s.io/apiserver/pkg/storage/names"
"k8s.io/client-go/kubernetes/scheme"
core "k8s.io/client-go/testing"
"sigs.k8s.io/cluster-api/pkg/apis"
"sigs.k8s.io/cluster-api/pkg/apis/cluster/common"
"sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha2"
"sigs.k8s.io/cluster-api/pkg/client/clientset_generated/clientset/fake"
fakeruntime "sigs.k8s.io/controller-runtime/pkg/client/fake"
)

func init() {
apis.AddToScheme(scheme.Scheme)
}

var (
infraResource = &unstructured.Unstructured{
Object: map[string]interface{}{
"kind": "InfrastructureRef",
"apiVersion": "infrastructure.cluster.sigs.k8s.io/v1alpha1",
"metadata": map[string]interface{}{
"name": "foo-template",
"namespace": "default",
},
"spec": map[string]interface{}{},
"status": map[string]interface{}{},
},
}
)

func addListMSReactor(fakeClient *fake.Clientset, obj runtime.Object) *fake.Clientset {
Expand Down Expand Up @@ -151,7 +175,13 @@ func generateDeployment(image string) v1alpha2.MachineDeployment {
ObjectMeta: v1alpha2.ObjectMeta{
Labels: machineLabels,
},
Spec: v1alpha2.MachineSpec{},
Spec: v1alpha2.MachineSpec{
InfrastructureRef: corev1.ObjectReference{
Name: infraResource.GetName(),
Kind: infraResource.GetKind(),
APIVersion: infraResource.GetAPIVersion(),
},
},
},
},
}
Expand All @@ -164,7 +194,13 @@ func generateMachineTemplateSpec(name, nodeName string, annotations, labels map[
Annotations: annotations,
Labels: labels,
},
Spec: v1alpha2.MachineSpec{},
Spec: v1alpha2.MachineSpec{
InfrastructureRef: corev1.ObjectReference{
Name: infraResource.GetName(),
Kind: infraResource.GetKind(),
APIVersion: infraResource.GetAPIVersion(),
},
},
}
}

Expand Down Expand Up @@ -307,7 +343,12 @@ func TestFindNewMachineSet(t *testing.T) {

for _, test := range tests {
t.Run(test.Name, func(t *testing.T) {
if ms := FindNewMachineSet(&test.deployment, test.msList); !reflect.DeepEqual(ms, test.expected) {
items := []runtime.Object{infraResource}
for _, ms := range test.msList {
items = append(items, ms)
}
c := fakeruntime.NewFakeClient(items...)
if ms := FindNewMachineSet(c, &test.deployment, test.msList); !reflect.DeepEqual(ms, test.expected) {
t.Errorf("In test case %q, expected %#v, got %#v", test.Name, test.expected, ms)
}
})
Expand Down Expand Up @@ -375,7 +416,12 @@ func TestFindOldMachineSets(t *testing.T) {

for _, test := range tests {
t.Run(test.Name, func(t *testing.T) {
requireMS, allMS := FindOldMachineSets(&test.deployment, test.msList)
items := []runtime.Object{infraResource}
for _, ms := range test.msList {
items = append(items, ms)
}
c := fakeruntime.NewFakeClient(items...)
requireMS, allMS := FindOldMachineSets(c, &test.deployment, test.msList)
sort.Sort(MachineSetsByCreationTimestamp(allMS))
sort.Sort(MachineSetsByCreationTimestamp(test.expected))
if !reflect.DeepEqual(allMS, test.expected) {
Expand Down