Skip to content

Commit

Permalink
Fix default namespace fallback (#209)
Browse files Browse the repository at this point in the history
* Add more logging
* Fix bug: was calling wrong IsNotFound func
* Remove confusing IsNotFound func from ibmcloud package
  • Loading branch information
JohnStarich authored Sep 22, 2020
1 parent ba68ead commit 3388d60
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 16 deletions.
14 changes: 9 additions & 5 deletions controllers/service_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client/fake"
)

var (
errNotFoundTest = fmt.Errorf("not found")
)

func TestService(t *testing.T) {
if testing.Short() {
t.SkipNow()
Expand Down Expand Up @@ -84,7 +88,7 @@ func TestService(t *testing.T) {
}, defaultWait, defaultTick)

_, err = getServiceInstanceFromObj(logger, serviceCopy)
assert.True(t, ibmcloud.IsNotFound(err), "Expect service to be deleted")
assert.Equal(t, errNotFoundTest, err, "Expect service to be deleted")
}
})
}
Expand Down Expand Up @@ -126,7 +130,7 @@ func TestService(t *testing.T) {
}, defaultWait, defaultTick)

_, err := getServiceInstanceFromObj(logger, serviceCopy)
assert.True(t, ibmcloud.IsNotFound(err))
assert.Equal(t, errNotFoundTest, err)
})

t.Run("should fail", func(t *testing.T) {
Expand Down Expand Up @@ -182,7 +186,7 @@ func getServiceInstanceFromObj(logt logr.Logger, service *ibmcloudv1.Service) (m
return instance, nil
}
}
return models.ServiceInstance{}, fmt.Errorf("not found")
return models.ServiceInstance{}, errNotFoundTest
}

func TestServiceV1Alpha1Compat(t *testing.T) {
Expand Down Expand Up @@ -212,7 +216,7 @@ func TestServiceV1Alpha1Compat(t *testing.T) {
}, defaultWait, defaultTick)

_, err = getServiceInstanceFromObj(logger, serviceCopy)
assert.True(t, ibmcloud.IsNotFound(err), "Expect service to be deleted")
assert.Equal(t, errNotFoundTest, err, "Expect service to be deleted")
}

func TestServiceV1Beta1Compat(t *testing.T) {
Expand Down Expand Up @@ -242,7 +246,7 @@ func TestServiceV1Beta1Compat(t *testing.T) {
}, defaultWait, defaultTick)

_, err = getServiceInstanceFromObj(logger, serviceCopy)
assert.True(t, ibmcloud.IsNotFound(err), "Expect service to be deleted")
assert.Equal(t, errNotFoundTest, err, "Expect service to be deleted")
}

func TestServiceLoadServiceFailed(t *testing.T) {
Expand Down
5 changes: 0 additions & 5 deletions internal/ibmcloud/errors.go

This file was deleted.

11 changes: 7 additions & 4 deletions internal/ibmcloud/ibmcloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
ibmcloudv1 "github.com/ibm/cloud-operators/api/v1"
"github.com/ibm/cloud-operators/internal/config"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -293,7 +294,7 @@ func getIamToken(logt logr.Logger, r client.Client, instance *ibmcloudv1.Service
}

func getConfigOrSecret(logt logr.Logger, r client.Client, instanceNamespace string, objName string, obj runtime.Object) error {
defaultNamespace, isManagement := getDefaultNamespace(r)
defaultNamespace, isManagement := getDefaultNamespace(logt, r)
if isManagement {
objName = instanceNamespace + "-" + objName
err := r.Get(context.TODO(), types.NamespacedName{Name: objName, Namespace: defaultNamespace}, obj)
Expand All @@ -305,14 +306,15 @@ func getConfigOrSecret(logt logr.Logger, r client.Client, instanceNamespace stri
}
err := r.Get(context.TODO(), types.NamespacedName{Name: objName, Namespace: instanceNamespace}, obj)
if err != nil {
if IsNotFound(err) {
if errors.IsNotFound(err) {
err = r.Get(context.TODO(), types.NamespacedName{Name: objName, Namespace: defaultNamespace}, obj)
if err != nil {
logt.Info("Unable to find secret or config in namespace", objName, defaultNamespace)
logt.Info("Unable to find secret or config in same namespace or default namespace", "secret or config", objName, "namespace", instanceNamespace, "default namespace", defaultNamespace)
return err
}
return nil
}
logt.Error(err, "Unknown error getting config or secret", "name", objName, "namespace", instanceNamespace)
return err
}
return nil
Expand Down Expand Up @@ -347,13 +349,14 @@ func getIBMCloudContext(instance *ibmcloudv1.Service, cm *v1.ConfigMap) ibmcloud
return instance.Spec.Context
}

func getDefaultNamespace(r client.Client) (string, bool) {
func getDefaultNamespace(logt logr.Logger, r client.Client) (string, bool) {
cm := &v1.ConfigMap{}
err := r.Get(context.Background(), types.NamespacedName{Namespace: config.Get().ControllerNamespace, Name: icoConfigMap}, cm)
if err != nil {
return "default", false
}

// There exists an ico-management configmap in the controller namespace
logt.Info("Found ibmcloud-operator-config in own namespace, using that namespace for configuration", "own namespace", config.Get().ControllerNamespace, "management namespace", cm.Data["namespace"])
return cm.Data["namespace"], true
}
5 changes: 3 additions & 2 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,12 @@ import (
"os"

"github.com/ibm/cloud-operators/controllers"
"go.uber.org/zap"
"k8s.io/apimachinery/pkg/runtime"
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
_ "k8s.io/client-go/plugin/pkg/client/auth/oidc"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/log/zap"
zapLog "sigs.k8s.io/controller-runtime/pkg/log/zap"

ibmcloudv1 "github.com/ibm/cloud-operators/api/v1"
ibmcloudv1alpha1 "github.com/ibm/cloud-operators/api/v1alpha1"
Expand Down Expand Up @@ -56,7 +57,7 @@ func main() {
"Enabling this will ensure there is only one active controller manager.")
flag.Parse()

ctrl.SetLogger(zap.New(zap.UseDevMode(true)))
ctrl.SetLogger(zapLog.New(zapLog.UseDevMode(true), zapLog.RawZapOpts(zap.AddCaller())))

mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{
Scheme: scheme,
Expand Down

0 comments on commit 3388d60

Please sign in to comment.