Merge pull request #6 from Nordix/bugfix-mael

🐛 Fix bug when cluster is not found

Author: metal3-io-bot

controllers/ippool_controller.go

@@ -30,11 +30,8 @@ import (
 	"sigs.k8s.io/cluster-api/util"
 	"sigs.k8s.io/cluster-api/util/patch"
 	ctrl "sigs.k8s.io/controller-runtime"
-	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/client"
-	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
-	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/source"
 )
 
@@ -118,7 +115,7 @@ func (r *IPPoolReconciler) Reconcile(req ctrl.Request) (_ ctrl.Result, rerr erro
 		return ctrl.Result{}, errors.Wrapf(err, "failed to create helper for managing the IP pool")
 	}
 
-	if ipamv1IPPool.Spec.ClusterName != nil && cluster != nil {
+	if ipamv1IPPool.Spec.ClusterName != nil && cluster != nil && cluster.Name != "" {
 		metadataLog = metadataLog.WithValues("cluster", cluster.Name)
 		if err := ipPoolMgr.SetClusterOwnerRef(cluster); err != nil {
 			return ctrl.Result{}, err
@@ -181,14 +178,6 @@ func (r *IPPoolReconciler) SetupWithManager(mgr ctrl.Manager) error {
 			&handler.EnqueueRequestsFromMapFunc{
 				ToRequests: handler.ToRequestsFunc(r.IPClaimToIPPool),
 			},
-			// Do not trigger a reconciliation on updates of the claim, as the Spec
-			// fields are immutable
-			builder.WithPredicates(predicate.Funcs{
-				UpdateFunc:  func(e event.UpdateEvent) bool { return false },
-				CreateFunc:  func(e event.CreateEvent) bool { return true },
-				DeleteFunc:  func(e event.DeleteEvent) bool { return true },
-				GenericFunc: func(e event.GenericEvent) bool { return false },
-			}),
 		).
 		Complete(r)
 }

controllers/ippool_controller_test.go

@@ -87,7 +87,9 @@ var _ = Describe("IPPool controller", func() {
 				if tc.setOwnerRefError {
 					m.EXPECT().SetClusterOwnerRef(gomock.Any()).Return(errors.New(""))
 				} else {
-					m.EXPECT().SetClusterOwnerRef(gomock.Any()).Return(nil)
+					if tc.cluster != nil {
+						m.EXPECT().SetClusterOwnerRef(gomock.Any()).Return(nil)
+					}
 				}
 			}
 

go.mod

@@ -10,6 +10,8 @@ require (
 	github.com/google/go-cmp v0.4.1 // indirect
 	github.com/hashicorp/golang-lru v0.5.4 // indirect
 	github.com/imdario/mergo v0.3.9 // indirect
+	github.com/onsi/ginkgo v1.12.0
+	github.com/onsi/gomega v1.9.0
 	github.com/opencontainers/go-digest v1.0.0 // indirect
 	github.com/pkg/errors v0.9.1
 	github.com/prometheus/client_golang v1.6.0 // indirect
@@ -21,7 +23,7 @@ require (
 	gomodules.xyz/jsonpatch/v2 v2.1.0 // indirect
 	gopkg.in/yaml.v2 v2.3.0 // indirect
 	k8s.io/api v0.19.0-alpha.2
-	k8s.io/apiextensions-apiserver v0.18.3 // indirect
+	k8s.io/apiextensions-apiserver v0.18.3
 	k8s.io/apimachinery v0.19.0-alpha.2
 	k8s.io/client-go v11.0.0+incompatible
 	k8s.io/klog v1.0.0

go.sum

@@ -302,6 +302,7 @@ github.com/go-logfmt/logfmt v0.3.0/go.mod h1:Qt1PoO58o5twSAckw1HlFXLmHsOX5/0LbT9
 github.com/go-logfmt/logfmt v0.4.0/go.mod h1:3RMwSq7FuexP4Kalkev3ejPJsZTpXXBr9+V4qmtdjCk=
 github.com/go-logr/logr v0.1.0 h1:M1Tv3VzNlEHg6uyACnRdtrploV2P7wZqH8BoQMtz0cg=
 github.com/go-logr/logr v0.1.0/go.mod h1:ixOQHD9gLJUVQQ2ZOR7zLEifBX6tGkNJF4QyIY7sIas=
+github.com/go-logr/zapr v0.1.0 h1:h+WVe9j6HAA01niTJPA/kKH0i7e0rLZBCwauQFcRE54=
 github.com/go-logr/zapr v0.1.0/go.mod h1:tabnROwaDl0UNxkVeFRbY8bwB37GwRv0P8lg6aAiEnk=
 github.com/go-logr/zapr v0.1.1 h1:qXBXPDdNncunGs7XeEpsJt8wCjYBygluzfdLO0G5baE=
 github.com/go-logr/zapr v0.1.1/go.mod h1:tabnROwaDl0UNxkVeFRbY8bwB37GwRv0P8lg6aAiEnk=
@@ -1540,6 +1541,7 @@ k8s.io/apiserver v0.17.3/go.mod h1:iJtsPpu1ZpEnHaNawpSV0nYTGBhhX2dUlnn7/QS7QiY=
 k8s.io/apiserver v0.18.0 h1:ELAWpGWC6XdbRLi5lwAbEbvksD7hkXxPdxaJsdpist4=
 k8s.io/apiserver v0.18.0/go.mod h1:3S2O6FeBBd6XTo0njUrLxiqk8GNy6wWOftjhJcXYnjw=
 k8s.io/apiserver v0.18.2/go.mod h1:Xbh066NqrZO8cbsoenCwyDJ1OSi8Ag8I2lezeHxzwzw=
+k8s.io/apiserver v0.18.3 h1:BVjccwKP/kEqY+ResOyWs0EKs7f4XL0d0E5GkU3uiqI=
 k8s.io/apiserver v0.18.3/go.mod h1:tHQRmthRPLUtwqsOnJJMoI8SW3lnoReZeE861lH8vUw=
 k8s.io/apiserver v0.19.0-alpha.2 h1:k1fpzJAPZvtRT9Z8Rc42kciGehIH0GiEmTgEmc46drw=
 k8s.io/apiserver v0.19.0-alpha.2/go.mod h1:3sm/OyOD8RpEaJ4B2MzqeGXsfVZP+T/7y+Hr0+0qfao=

ipam/ippool_manager.go

@@ -206,7 +206,6 @@ func (m *IPPoolManager) updateAddress(ctx context.Context,
 	}
 	// Always patch addressClaim exiting this function so we can persist any changes.
 	defer func() {
-		fmt.Printf("\nPatching %v", addressClaim.Name)
 		err := helper.Patch(ctx, addressClaim)
 		if err != nil {
 			m.Log.Info("failed to Patch IPClaim")
@@ -253,7 +252,6 @@ func (m *IPPoolManager) allocateAddress(addressClaim *ipamv1.IPClaim,
 		index := 0
 		for !ipAllocated {
 			allocatedAddress, err = getIPAddress(pool, index)
-			fmt.Println(allocatedAddress)
 			if err != nil {
 				break
 			}
@@ -306,6 +304,22 @@ func (m *IPPoolManager) createAddress(ctx context.Context,
 
 	m.Log.Info("Address allocated", "Claim", addressClaim.Name, "address", allocatedAddress)
 
+	ownerRefs := addressClaim.OwnerReferences
+	ownerRefs = append(ownerRefs,
+		metav1.OwnerReference{
+			APIVersion: m.IPPool.APIVersion,
+			Kind:       m.IPPool.Kind,
+			Name:       m.IPPool.Name,
+			UID:        m.IPPool.UID,
+		},
+		metav1.OwnerReference{
+			APIVersion: addressClaim.APIVersion,
+			Kind:       addressClaim.Kind,
+			Name:       addressClaim.Name,
+			UID:        addressClaim.UID,
+		},
+	)
+
 	// Create the IPAddress object, with an Owner ref to the Metal3Machine
 	// (curOwnerRef) and to the IPPool
 	addressObject := &ipamv1.IPAddress{
@@ -314,24 +328,10 @@ func (m *IPPoolManager) createAddress(ctx context.Context,
 			APIVersion: ipamv1.GroupVersion.String(),
 		},
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      addressName,
-			Namespace: m.IPPool.Namespace,
-			OwnerReferences: []metav1.OwnerReference{
-				metav1.OwnerReference{
-					Controller: pointer.BoolPtr(true),
-					APIVersion: m.IPPool.APIVersion,
-					Kind:       m.IPPool.Kind,
-					Name:       m.IPPool.Name,
-					UID:        m.IPPool.UID,
-				},
-				metav1.OwnerReference{
-					APIVersion: addressClaim.APIVersion,
-					Kind:       addressClaim.Kind,
-					Name:       addressClaim.Name,
-					UID:        addressClaim.UID,
-				},
-			},
-			Labels: addressClaim.Labels,
+			Name:            addressName,
+			Namespace:       m.IPPool.Namespace,
+			OwnerReferences: ownerRefs,
+			Labels:          addressClaim.Labels,
 		},
 		Spec: ipamv1.IPAddressSpec{
 			Address: allocatedAddress,