diff --git a/federation/pkg/federation-controller/daemonset/daemonset_controller.go b/federation/pkg/federation-controller/daemonset/daemonset_controller.go index 8614d1d332c..f27007887ef 100644 --- a/federation/pkg/federation-controller/daemonset/daemonset_controller.go +++ b/federation/pkg/federation-controller/daemonset/daemonset_controller.go @@ -239,20 +239,20 @@ func (daemonsetcontroller *DaemonSetController) removeFinalizerFunc(obj pkgrunti return daemonset, nil } -// Adds the given finalizer to the given objects ObjectMeta. +// Adds the given finalizers to the given objects ObjectMeta. // Assumes that the given object is a daemonset. -func (daemonsetcontroller *DaemonSetController) addFinalizerFunc(obj pkgruntime.Object, finalizer string) (pkgruntime.Object, error) { +func (daemonsetcontroller *DaemonSetController) addFinalizerFunc(obj pkgruntime.Object, finalizers []string) (pkgruntime.Object, error) { daemonset := obj.(*extensionsv1.DaemonSet) - daemonset.ObjectMeta.Finalizers = append(daemonset.ObjectMeta.Finalizers, finalizer) + daemonset.ObjectMeta.Finalizers = append(daemonset.ObjectMeta.Finalizers, finalizers...) daemonset, err := daemonsetcontroller.federatedApiClient.Extensions().DaemonSets(daemonset.Namespace).Update(daemonset) if err != nil { - return nil, fmt.Errorf("failed to add finalizer %s to daemonset %s: %v", finalizer, daemonset.Name, err) + return nil, fmt.Errorf("failed to add finalizers %v to daemonset %s: %v", finalizers, daemonset.Name, err) } return daemonset, nil } func (daemonsetcontroller *DaemonSetController) Run(stopChan <-chan struct{}) { - glog.V(1).Infof("Starting daemonset controllr") + glog.V(1).Infof("Starting daemonset controller") go daemonsetcontroller.daemonsetInformerController.Run(stopChan) glog.V(1).Infof("Starting daemonset federated informer") diff --git a/federation/pkg/federation-controller/daemonset/daemonset_controller_test.go b/federation/pkg/federation-controller/daemonset/daemonset_controller_test.go index f5cfa486e2c..3ac9052ba89 100644 --- a/federation/pkg/federation-controller/daemonset/daemonset_controller_test.go +++ b/federation/pkg/federation-controller/daemonset/daemonset_controller_test.go @@ -97,10 +97,9 @@ func TestDaemonSetController(t *testing.T) { // Test add federated daemonset. daemonsetWatch.Add(&daemonset1) - // There should be 2 updates to add both the finalizers. + // There should be an update to add both the finalizers. updatedDaemonSet := GetDaemonSetFromChan(daemonsetUpdateChan) assert.True(t, daemonsetController.hasFinalizerFunc(updatedDaemonSet, deletionhelper.FinalizerDeleteFromUnderlyingClusters)) - updatedDaemonSet = GetDaemonSetFromChan(daemonsetUpdateChan) assert.True(t, daemonsetController.hasFinalizerFunc(updatedDaemonSet, metav1.FinalizerOrphan)) daemonset1 = *updatedDaemonSet diff --git a/federation/pkg/federation-controller/deployment/deploymentcontroller.go b/federation/pkg/federation-controller/deployment/deploymentcontroller.go index 5ec1bb53c2b..7a6d23c0168 100644 --- a/federation/pkg/federation-controller/deployment/deploymentcontroller.go +++ b/federation/pkg/federation-controller/deployment/deploymentcontroller.go @@ -256,14 +256,14 @@ func (fdc *DeploymentController) removeFinalizerFunc(obj runtime.Object, finaliz return deployment, nil } -// Adds the given finalizer to the given objects ObjectMeta. +// Adds the given finalizers to the given objects ObjectMeta. // Assumes that the given object is a deployment. -func (fdc *DeploymentController) addFinalizerFunc(obj runtime.Object, finalizer string) (runtime.Object, error) { +func (fdc *DeploymentController) addFinalizerFunc(obj runtime.Object, finalizers []string) (runtime.Object, error) { deployment := obj.(*extensionsv1.Deployment) - deployment.ObjectMeta.Finalizers = append(deployment.ObjectMeta.Finalizers, finalizer) + deployment.ObjectMeta.Finalizers = append(deployment.ObjectMeta.Finalizers, finalizers...) deployment, err := fdc.fedClient.Extensions().Deployments(deployment.Namespace).Update(deployment) if err != nil { - return nil, fmt.Errorf("failed to add finalizer %s to deployment %s: %v", finalizer, deployment.Name, err) + return nil, fmt.Errorf("failed to add finalizers %v to deployment %s: %v", finalizers, deployment.Name, err) } return deployment, nil } @@ -420,8 +420,10 @@ func (fdc *DeploymentController) schedule(fd *extensionsv1.Deployment, clusters if fdPref != nil { // create a new planner if user specified a preference plannerToBeUsed = planner.NewPlanner(fdPref) } - - replicas := int64(*fd.Spec.Replicas) + replicas := int64(0) + if fd.Spec.Replicas != nil { + replicas = int64(*fd.Spec.Replicas) + } var clusterNames []string for _, cluster := range clusters { clusterNames = append(clusterNames, cluster.Name) diff --git a/federation/pkg/federation-controller/deployment/deploymentcontroller_test.go b/federation/pkg/federation-controller/deployment/deploymentcontroller_test.go index 2fd6d92d8e5..7a857fde9aa 100644 --- a/federation/pkg/federation-controller/deployment/deploymentcontroller_test.go +++ b/federation/pkg/federation-controller/deployment/deploymentcontroller_test.go @@ -85,6 +85,9 @@ func TestDeploymentController(t *testing.T) { cluster2 := NewCluster("cluster2", apiv1.ConditionTrue) fakeClient := &fakefedclientset.Clientset{} + // Add an update reactor on fake client to return the desired updated object. + // This is a hack to workaround https://github.com/kubernetes/kubernetes/issues/40939. + AddFakeUpdateReactor("deployments", &fakeClient.Fake) RegisterFakeList("clusters", &fakeClient.Fake, &fedv1.ClusterList{Items: []fedv1.Cluster{*cluster1}}) deploymentsWatch := RegisterFakeWatch("deployments", &fakeClient.Fake) clusterWatch := RegisterFakeWatch("clusters", &fakeClient.Fake) diff --git a/federation/pkg/federation-controller/ingress/ingress_controller.go b/federation/pkg/federation-controller/ingress/ingress_controller.go index 0fee0140fdc..2dc762a349d 100644 --- a/federation/pkg/federation-controller/ingress/ingress_controller.go +++ b/federation/pkg/federation-controller/ingress/ingress_controller.go @@ -339,14 +339,14 @@ func (ic *IngressController) removeFinalizerFunc(obj pkgruntime.Object, finalize return ingress, nil } -// Adds the given finalizer to the given objects ObjectMeta. +// Adds the given finalizers to the given objects ObjectMeta. // Assumes that the given object is a ingress. -func (ic *IngressController) addFinalizerFunc(obj pkgruntime.Object, finalizer string) (pkgruntime.Object, error) { +func (ic *IngressController) addFinalizerFunc(obj pkgruntime.Object, finalizers []string) (pkgruntime.Object, error) { ingress := obj.(*extensionsv1beta1.Ingress) - ingress.ObjectMeta.Finalizers = append(ingress.ObjectMeta.Finalizers, finalizer) + ingress.ObjectMeta.Finalizers = append(ingress.ObjectMeta.Finalizers, finalizers...) ingress, err := ic.federatedApiClient.Extensions().Ingresses(ingress.Namespace).Update(ingress) if err != nil { - return nil, fmt.Errorf("failed to add finalizer %s to ingress %s: %v", finalizer, ingress.Name, err) + return nil, fmt.Errorf("failed to add finalizers %v to ingress %s: %v", finalizers, ingress.Name, err) } return ingress, nil } diff --git a/federation/pkg/federation-controller/ingress/ingress_controller_test.go b/federation/pkg/federation-controller/ingress/ingress_controller_test.go index 8d19afe6f9c..132532a447d 100644 --- a/federation/pkg/federation-controller/ingress/ingress_controller_test.go +++ b/federation/pkg/federation-controller/ingress/ingress_controller_test.go @@ -140,10 +140,9 @@ func TestIngressController(t *testing.T) { assert.Equal(t, cluster.ObjectMeta.Annotations[uidAnnotationKey], cfg1.Data[uidKey]) t.Logf("Checking that appropriate finalizers are added") - // There should be 2 updates to add both the finalizers. + // There should be an update to add both the finalizers. updatedIngress := GetIngressFromChan(t, fedIngressUpdateChan) assert.True(t, ingressController.hasFinalizerFunc(updatedIngress, deletionhelper.FinalizerDeleteFromUnderlyingClusters)) - updatedIngress = GetIngressFromChan(t, fedIngressUpdateChan) assert.True(t, ingressController.hasFinalizerFunc(updatedIngress, metav1.FinalizerOrphan), fmt.Sprintf("ingress does not have the orphan finalizer: %v", updatedIngress)) fedIngress = *updatedIngress diff --git a/federation/pkg/federation-controller/namespace/namespace_controller.go b/federation/pkg/federation-controller/namespace/namespace_controller.go index 0644dac201d..b74d0e8807f 100644 --- a/federation/pkg/federation-controller/namespace/namespace_controller.go +++ b/federation/pkg/federation-controller/namespace/namespace_controller.go @@ -219,14 +219,14 @@ func (nc *NamespaceController) removeFinalizerFunc(obj runtime.Object, finalizer return namespace, nil } -// Adds the given finalizer to the given objects ObjectMeta. +// Adds the given finalizers to the given objects ObjectMeta. // Assumes that the given object is a namespace. -func (nc *NamespaceController) addFinalizerFunc(obj runtime.Object, finalizer string) (runtime.Object, error) { +func (nc *NamespaceController) addFinalizerFunc(obj runtime.Object, finalizers []string) (runtime.Object, error) { namespace := obj.(*apiv1.Namespace) - namespace.ObjectMeta.Finalizers = append(namespace.ObjectMeta.Finalizers, finalizer) + namespace.ObjectMeta.Finalizers = append(namespace.ObjectMeta.Finalizers, finalizers...) namespace, err := nc.federatedApiClient.Core().Namespaces().Finalize(namespace) if err != nil { - return nil, fmt.Errorf("failed to add finalizer %s to namespace %s: %v", finalizer, namespace.Name, err) + return nil, fmt.Errorf("failed to add finalizers %v to namespace %s: %v", finalizers, namespace.Name, err) } return namespace, nil } diff --git a/federation/pkg/federation-controller/replicaset/replicasetcontroller.go b/federation/pkg/federation-controller/replicaset/replicasetcontroller.go index 5e4bde06e41..c31b1c38350 100644 --- a/federation/pkg/federation-controller/replicaset/replicasetcontroller.go +++ b/federation/pkg/federation-controller/replicaset/replicasetcontroller.go @@ -260,14 +260,14 @@ func (frsc *ReplicaSetController) removeFinalizerFunc(obj runtime.Object, finali return replicaset, nil } -// Adds the given finalizer to the given objects ObjectMeta. +// Adds the given finalizers to the given objects ObjectMeta. // Assumes that the given object is a replicaset. -func (frsc *ReplicaSetController) addFinalizerFunc(obj runtime.Object, finalizer string) (runtime.Object, error) { +func (frsc *ReplicaSetController) addFinalizerFunc(obj runtime.Object, finalizers []string) (runtime.Object, error) { replicaset := obj.(*extensionsv1.ReplicaSet) - replicaset.ObjectMeta.Finalizers = append(replicaset.ObjectMeta.Finalizers, finalizer) + replicaset.ObjectMeta.Finalizers = append(replicaset.ObjectMeta.Finalizers, finalizers...) replicaset, err := frsc.fedClient.Extensions().ReplicaSets(replicaset.Namespace).Update(replicaset) if err != nil { - return nil, fmt.Errorf("failed to add finalizer %s to replicaset %s: %v", finalizer, replicaset.Name, err) + return nil, fmt.Errorf("failed to add finalizers %v to replicaset %s: %v", finalizers, replicaset.Name, err) } return replicaset, nil } diff --git a/federation/pkg/federation-controller/secret/secret_controller.go b/federation/pkg/federation-controller/secret/secret_controller.go index a4962aed8ab..a2419d93a2d 100644 --- a/federation/pkg/federation-controller/secret/secret_controller.go +++ b/federation/pkg/federation-controller/secret/secret_controller.go @@ -220,14 +220,14 @@ func (secretcontroller *SecretController) removeFinalizerFunc(obj pkgruntime.Obj return secret, nil } -// Adds the given finalizer to the given objects ObjectMeta. +// Adds the given finalizers to the given objects ObjectMeta. // Assumes that the given object is a secret. -func (secretcontroller *SecretController) addFinalizerFunc(obj pkgruntime.Object, finalizer string) (pkgruntime.Object, error) { +func (secretcontroller *SecretController) addFinalizerFunc(obj pkgruntime.Object, finalizers []string) (pkgruntime.Object, error) { secret := obj.(*apiv1.Secret) - secret.ObjectMeta.Finalizers = append(secret.ObjectMeta.Finalizers, finalizer) + secret.ObjectMeta.Finalizers = append(secret.ObjectMeta.Finalizers, finalizers...) secret, err := secretcontroller.federatedApiClient.Core().Secrets(secret.Namespace).Update(secret) if err != nil { - return nil, fmt.Errorf("failed to add finalizer %s to secret %s: %v", finalizer, secret.Name, err) + return nil, fmt.Errorf("failed to add finalizers %v to secret %s: %v", finalizers, secret.Name, err) } return secret, nil } diff --git a/federation/pkg/federation-controller/secret/secret_controller_test.go b/federation/pkg/federation-controller/secret/secret_controller_test.go index 4fea99e5da4..24b790dad2e 100644 --- a/federation/pkg/federation-controller/secret/secret_controller_test.go +++ b/federation/pkg/federation-controller/secret/secret_controller_test.go @@ -97,10 +97,9 @@ func TestSecretController(t *testing.T) { // Test add federated secret. secretWatch.Add(&secret1) - // There should be 2 updates to add both the finalizers. + // There should be an update to add both the finalizers. updatedSecret := GetSecretFromChan(secretUpdateChan) assert.True(t, secretController.hasFinalizerFunc(updatedSecret, deletionhelper.FinalizerDeleteFromUnderlyingClusters)) - updatedSecret = GetSecretFromChan(secretUpdateChan) assert.True(t, secretController.hasFinalizerFunc(updatedSecret, metav1.FinalizerOrphan)) secret1 = *updatedSecret diff --git a/federation/pkg/federation-controller/service/servicecontroller.go b/federation/pkg/federation-controller/service/servicecontroller.go index 278ce5681d7..5c07aa46c94 100644 --- a/federation/pkg/federation-controller/service/servicecontroller.go +++ b/federation/pkg/federation-controller/service/servicecontroller.go @@ -345,14 +345,14 @@ func (s *ServiceController) removeFinalizerFunc(obj pkgruntime.Object, finalizer return service, nil } -// Adds the given finalizer to the given objects ObjectMeta. +// Adds the given finalizers to the given objects ObjectMeta. // Assumes that the given object is a service. -func (s *ServiceController) addFinalizerFunc(obj pkgruntime.Object, finalizer string) (pkgruntime.Object, error) { +func (s *ServiceController) addFinalizerFunc(obj pkgruntime.Object, finalizers []string) (pkgruntime.Object, error) { service := obj.(*v1.Service) - service.ObjectMeta.Finalizers = append(service.ObjectMeta.Finalizers, finalizer) + service.ObjectMeta.Finalizers = append(service.ObjectMeta.Finalizers, finalizers...) service, err := s.federationClient.Core().Services(service.Namespace).Update(service) if err != nil { - return nil, fmt.Errorf("failed to add finalizer %s to service %s: %v", finalizer, service.Name, err) + return nil, fmt.Errorf("failed to add finalizers %v to service %s: %v", finalizers, service.Name, err) } return service, nil } diff --git a/federation/pkg/federation-controller/util/deletionhelper/deletion_helper.go b/federation/pkg/federation-controller/util/deletionhelper/deletion_helper.go index 3d6b7e97d77..528f017edfa 100644 --- a/federation/pkg/federation-controller/util/deletionhelper/deletion_helper.go +++ b/federation/pkg/federation-controller/util/deletionhelper/deletion_helper.go @@ -46,7 +46,7 @@ const ( type HasFinalizerFunc func(runtime.Object, string) bool type RemoveFinalizerFunc func(runtime.Object, string) (runtime.Object, error) -type AddFinalizerFunc func(runtime.Object, string) (runtime.Object, error) +type AddFinalizerFunc func(runtime.Object, []string) (runtime.Object, error) type ObjNameFunc func(runtime.Object) string type DeletionHelper struct { @@ -89,19 +89,16 @@ func NewDeletionHelper( // This method should be called before creating objects in underlying clusters. func (dh *DeletionHelper) EnsureFinalizers(obj runtime.Object) ( runtime.Object, error) { + finalizers := []string{} if !dh.hasFinalizerFunc(obj, FinalizerDeleteFromUnderlyingClusters) { - glog.V(2).Infof("Adding finalizer %s to %s", FinalizerDeleteFromUnderlyingClusters, dh.objNameFunc(obj)) - obj, err := dh.addFinalizerFunc(obj, FinalizerDeleteFromUnderlyingClusters) - if err != nil { - return obj, err - } + finalizers = append(finalizers, FinalizerDeleteFromUnderlyingClusters) } if !dh.hasFinalizerFunc(obj, metav1.FinalizerOrphan) { - glog.V(2).Infof("Adding finalizer %s to %s", metav1.FinalizerOrphan, dh.objNameFunc(obj)) - obj, err := dh.addFinalizerFunc(obj, metav1.FinalizerOrphan) - if err != nil { - return obj, err - } + finalizers = append(finalizers, metav1.FinalizerOrphan) + } + if len(finalizers) != 0 { + glog.V(2).Infof("Adding finalizers %v to %s", finalizers, dh.objNameFunc(obj)) + return dh.addFinalizerFunc(obj, finalizers) } return obj, nil } diff --git a/federation/pkg/federation-controller/util/test/test_helper.go b/federation/pkg/federation-controller/util/test/test_helper.go index 409903f6b56..4a9020725e5 100644 --- a/federation/pkg/federation-controller/util/test/test_helper.go +++ b/federation/pkg/federation-controller/util/test/test_helper.go @@ -226,6 +226,18 @@ func RegisterFakeCopyOnUpdate(resource string, client *core.Fake, watcher *Watch return objChan } +// Adds an update reactor to the given fake client. +// The reactor just returns the object passed to update action. +// This is used as a hack to workaround https://github.com/kubernetes/kubernetes/issues/40939. +// Without this, all update actions using fake client return empty objects. +func AddFakeUpdateReactor(resource string, client *core.Fake) { + client.AddReactor("update", resource, func(action core.Action) (bool, runtime.Object, error) { + updateAction := action.(core.UpdateAction) + originalObj := updateAction.GetObject() + return true, originalObj, nil + }) +} + // GetObjectFromChan tries to get an api object from the given channel // within a reasonable time. func GetObjectFromChan(c chan runtime.Object) runtime.Object {