Merge pull request #20300 from deads2k/rebase-05-origin-finalizer

strip off the old openshift namespace finalizer

Author: openshift-merge-robot

pkg/oc/cli/cmd/project.go

@@ -1,7 +1,6 @@
 package cmd
 
 import (
-	"bytes"
 	"errors"
 	"fmt"
 	"net/url"
@@ -358,18 +357,6 @@ func getProjects(projectClient projectclient.ProjectInterface, kClient kclientse
 	return projects.Items, nil
 }
 
-func clusterAndAuthEquality(clientCfg *restclient.Config, cluster clientcmdapi.Cluster, authInfo clientcmdapi.AuthInfo) bool {
-	return cluster.Server == clientCfg.Host &&
-		cluster.InsecureSkipTLSVerify == clientCfg.Insecure &&
-		cluster.CertificateAuthority == clientCfg.CAFile &&
-		bytes.Equal(cluster.CertificateAuthorityData, clientCfg.CAData) &&
-		authInfo.Token == clientCfg.BearerToken &&
-		authInfo.ClientCertificate == clientCfg.TLSClientConfig.CertFile &&
-		bytes.Equal(authInfo.ClientCertificateData, clientCfg.TLSClientConfig.CertData) &&
-		authInfo.ClientKey == clientCfg.TLSClientConfig.KeyFile &&
-		bytes.Equal(authInfo.ClientKeyData, clientCfg.TLSClientConfig.KeyData)
-}
-
 // TODO these kind of funcs could be moved to some kind of clientcmd util
 func hasMultipleServers(config clientcmdapi.Config) bool {
 	server := ""

pkg/project/controller/project_finalizer_controller.go

@@ -2,43 +2,44 @@ package controller
 
 import (
 	"fmt"
-	"time"
+
+	"github.com/golang/glog"
 
 	"k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/util/runtime"
-	informers "k8s.io/client-go/informers/core/v1"
-	kclientset "k8s.io/client-go/kubernetes"
+	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
+	"k8s.io/apimachinery/pkg/util/sets"
+	corev1informers "k8s.io/client-go/informers/core/v1"
+	"k8s.io/client-go/kubernetes"
+	corev1listers "k8s.io/client-go/listers/core/v1"
 	"k8s.io/client-go/tools/cache"
 	"k8s.io/client-go/util/workqueue"
 
-	"github.com/golang/glog"
-	projectutil "github.com/openshift/origin/pkg/project/util"
+	projectapiv1 "github.com/openshift/api/project/v1"
 )
 
 // ProjectFinalizerController is responsible for participating in Kubernetes Namespace termination
 type ProjectFinalizerController struct {
-	client kclientset.Interface
+	client kubernetes.Interface
 
-	queue      workqueue.RateLimitingInterface
-	maxRetries int
+	queue workqueue.RateLimitingInterface
 
-	controller cache.Controller
-	cache      cache.Store
+	cacheSynced cache.InformerSynced
+	nsLister    corev1listers.NamespaceLister
 
 	// extracted for testing
 	syncHandler func(key string) error
 }
 
-func NewProjectFinalizerController(namespaces informers.NamespaceInformer, client kclientset.Interface) *ProjectFinalizerController {
+func NewProjectFinalizerController(namespaces corev1informers.NamespaceInformer, client kubernetes.Interface) *ProjectFinalizerController {
 	c := &ProjectFinalizerController{
-		client:     client,
-		controller: namespaces.Informer().GetController(),
-		cache:      namespaces.Informer().GetStore(),
-		queue:      workqueue.NewRateLimitingQueue(workqueue.DefaultControllerRateLimiter()),
-		maxRetries: 10,
+		client:      client,
+		cacheSynced: namespaces.Informer().HasSynced,
+		queue:       workqueue.NewRateLimitingQueue(workqueue.DefaultControllerRateLimiter()),
+		nsLister:    namespaces.Lister(),
 	}
-	namespaces.Informer().AddEventHandlerWithResyncPeriod(
-		// TODO: generalize naiveResourceEventHandler and use it here
+	namespaces.Informer().AddEventHandler(
 		cache.ResourceEventHandlerFuncs{
 			AddFunc: func(obj interface{}) {
 				c.enqueueNamespace(obj)
@@ -47,7 +48,6 @@ func NewProjectFinalizerController(namespaces informers.NamespaceInformer, clien
 				c.enqueueNamespace(newObj)
 			},
 		},
-		10*time.Minute,
 	)
 
 	c.syncHandler = c.syncNamespace
@@ -60,7 +60,7 @@ func (c *ProjectFinalizerController) Run(stopCh <-chan struct{}, workers int) {
 	defer c.queue.ShutDown()
 
 	// Wait for the stores to fill
-	if !cache.WaitForCacheSync(stopCh, c.controller.HasSynced) {
+	if !cache.WaitForCacheSync(stopCh, c.cacheSynced) {
 		return
 	}
 
@@ -73,11 +73,12 @@ func (c *ProjectFinalizerController) Run(stopCh <-chan struct{}, workers int) {
 }
 
 func (c *ProjectFinalizerController) enqueueNamespace(obj interface{}) {
-	ns, ok := obj.(*v1.Namespace)
-	if !ok {
+	key, err := cache.DeletionHandlingMetaNamespaceKeyFunc(obj)
+	if err != nil {
+		utilruntime.HandleError(err)
 		return
 	}
-	c.queue.Add(ns.Name)
+	c.queue.Add(key)
 }
 
 // worker runs a worker thread that just dequeues items, processes them, and marks them done.
@@ -113,29 +114,41 @@ func (c *ProjectFinalizerController) work() bool {
 // syncNamespace will sync the namespace with the given key.
 // This function is not meant to be invoked concurrently with the same key.
 func (c *ProjectFinalizerController) syncNamespace(key string) error {
-	item, exists, err := c.cache.GetByKey(key)
+	ns, err := c.nsLister.Get(key)
+	if errors.IsNotFound(err) {
+		return nil
+	}
 	if err != nil {
 		return err
 	}
-	if !exists {
+
+	found := false
+	for _, finalizerName := range ns.Spec.Finalizers {
+		if projectapiv1.FinalizerOrigin == finalizerName {
+			found = true
+		}
+	}
+	if !found {
 		return nil
 	}
-	return c.finalize(item.(*v1.Namespace))
+
+	return c.finalize(ns.DeepCopy())
 }
 
 // finalize processes a namespace and deletes content in origin if its terminating
 func (c *ProjectFinalizerController) finalize(namespace *v1.Namespace) error {
-	// if namespace is not terminating, ignore it
-	if namespace.Status.Phase != v1.NamespaceTerminating {
-		return nil
+	finalizerSet := sets.NewString()
+	for i := range namespace.Spec.Finalizers {
+		finalizerSet.Insert(string(namespace.Spec.Finalizers[i]))
 	}
+	finalizerSet.Delete(string(projectapiv1.FinalizerOrigin))
 
-	// if we already processed this namespace, ignore it
-	if projectutil.Finalized(namespace) {
-		return nil
+	namespace.Spec.Finalizers = make([]v1.FinalizerName, 0, len(finalizerSet))
+	for _, value := range finalizerSet.List() {
+		namespace.Spec.Finalizers = append(namespace.Spec.Finalizers, v1.FinalizerName(value))
 	}
 
 	// we have removed content, so mark it finalized by us
-	_, err := projectutil.Finalize(c.client, namespace)
+	_, err := c.client.Core().Namespaces().Finalize(namespace)
 	return err
 }

pkg/project/controller/project_finalizer_controller_test.go

@@ -6,10 +6,10 @@ import (
 	"k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/client-go/kubernetes/fake"
 	clientgotesting "k8s.io/client-go/testing"
 
 	projectapiv1 "github.com/openshift/api/project/v1"
-	"k8s.io/client-go/kubernetes/fake"
 )
 
 func TestSyncNamespaceThatIsTerminating(t *testing.T) {
@@ -74,12 +74,18 @@ func TestSyncNamespaceThatIsActive(t *testing.T) {
 	if err != nil {
 		t.Errorf("Unexpected error when handling namespace %v", err)
 	}
+	expectedActionSet := []clientgotesting.Action{
+		clientgotesting.NewListAction(
+			schema.GroupVersionResource{Group: "", Version: "v1", Resource: "namespace"},
+			schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Namespace"},
+			"", metav1.ListOptions{}),
+	}
 	kubeActionSet := []clientgotesting.Action{}
 	for i := range mockKubeClient.Actions() {
 		kubeActionSet = append(kubeActionSet, mockKubeClient.Actions()[i])
 	}
 
-	if (len(kubeActionSet)) != 0 {
-		t.Errorf("Expected no actions from contoller, but got: %#v", kubeActionSet)
+	if (len(kubeActionSet)) != len(expectedActionSet) {
+		t.Errorf("Expected actions: %v, but got: %v", expectedActionSet, kubeActionSet)
 	}
 }

pkg/project/util/util.go

@@ -1,122 +1,11 @@
 package util
 
 import (
-	"k8s.io/api/core/v1"
-	kerrors "k8s.io/apimachinery/pkg/api/errors"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/util/sets"
-	clientset "k8s.io/client-go/kubernetes"
-	kapi "k8s.io/kubernetes/pkg/apis/core"
-	"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
-
-	projectapiv1 "github.com/openshift/api/project/v1"
 	oapi "github.com/openshift/origin/pkg/api"
 	projectapi "github.com/openshift/origin/pkg/project/apis/project"
+	kapi "k8s.io/kubernetes/pkg/apis/core"
 )
 
-// Associated returns true if the spec.finalizers contains the origin finalizer
-func Associated(namespace *kapi.Namespace) bool {
-	for i := range namespace.Spec.Finalizers {
-		if projectapi.FinalizerOrigin == namespace.Spec.Finalizers[i] {
-			return true
-		}
-	}
-	return false
-}
-
-// Associate adds the origin finalizer to spec.finalizers if its not there already
-func Associate(kubeClient internalclientset.Interface, namespace *kapi.Namespace) (*kapi.Namespace, error) {
-	if Associated(namespace) {
-		return namespace, nil
-	}
-	return finalizeInternal(kubeClient, namespace, true)
-}
-
-// Finalized returns true if the spec.finalizers does not contain the origin finalizer
-func Finalized(namespace *v1.Namespace) bool {
-	for i := range namespace.Spec.Finalizers {
-		if projectapiv1.FinalizerOrigin == namespace.Spec.Finalizers[i] {
-			return false
-		}
-	}
-	return true
-}
-
-// Finalize will remove the origin finalizer from the namespace
-func Finalize(kubeClient clientset.Interface, namespace *v1.Namespace) (result *v1.Namespace, err error) {
-	if Finalized(namespace) {
-		return namespace, nil
-	}
-
-	// there is a potential for a resource conflict with base kubernetes finalizer
-	// as a result, we handle resource conflicts in case multiple finalizers try
-	// to finalize at same time
-	for {
-		result, err = finalizeInternalV1(kubeClient, namespace, false)
-		if err == nil {
-			return result, nil
-		}
-
-		if !kerrors.IsConflict(err) {
-			return nil, err
-		}
-
-		namespace, err = kubeClient.Core().Namespaces().Get(namespace.Name, metav1.GetOptions{})
-		if err != nil {
-			return nil, err
-		}
-	}
-}
-
-// finalizeInternal will update the namespace finalizer list to either have or not have origin finalizer
-// TODO: remove me
-func finalizeInternal(kubeClient internalclientset.Interface, namespace *kapi.Namespace, withOrigin bool) (*kapi.Namespace, error) {
-	namespaceFinalize := kapi.Namespace{}
-	namespaceFinalize.ObjectMeta = namespace.ObjectMeta
-	namespaceFinalize.Spec = namespace.Spec
-
-	finalizerSet := sets.NewString()
-	for i := range namespace.Spec.Finalizers {
-		finalizerSet.Insert(string(namespace.Spec.Finalizers[i]))
-	}
-
-	if withOrigin {
-		finalizerSet.Insert(string(projectapi.FinalizerOrigin))
-	} else {
-		finalizerSet.Delete(string(projectapi.FinalizerOrigin))
-	}
-
-	namespaceFinalize.Spec.Finalizers = make([]kapi.FinalizerName, 0, len(finalizerSet))
-	for _, value := range finalizerSet.List() {
-		namespaceFinalize.Spec.Finalizers = append(namespaceFinalize.Spec.Finalizers, kapi.FinalizerName(value))
-	}
-	return kubeClient.Core().Namespaces().Finalize(&namespaceFinalize)
-}
-
-// finalizeInternalV1 will update the namespace finalizer list to either have or not have origin finalizer
-func finalizeInternalV1(kubeClient clientset.Interface, namespace *v1.Namespace, withOrigin bool) (*v1.Namespace, error) {
-	namespaceFinalize := v1.Namespace{}
-	namespaceFinalize.ObjectMeta = namespace.ObjectMeta
-	namespaceFinalize.Spec = namespace.Spec
-
-	finalizerSet := sets.NewString()
-	for i := range namespace.Spec.Finalizers {
-		finalizerSet.Insert(string(namespace.Spec.Finalizers[i]))
-	}
-
-	if withOrigin {
-		finalizerSet.Insert(string(projectapiv1.FinalizerOrigin))
-	} else {
-		finalizerSet.Delete(string(projectapiv1.FinalizerOrigin))
-	}
-
-	namespaceFinalize.Spec.Finalizers = make([]v1.FinalizerName, 0, len(finalizerSet))
-	for _, value := range finalizerSet.List() {
-		namespaceFinalize.Spec.Finalizers = append(namespaceFinalize.Spec.Finalizers, v1.FinalizerName(value))
-	}
-	return kubeClient.Core().Namespaces().Finalize(&namespaceFinalize)
-}
-
 // ConvertNamespace transforms a Namespace into a Project
 func ConvertNamespace(namespace *kapi.Namespace) *projectapi.Project {
 	return &projectapi.Project{