adding consolelink namespacedashboard #13

akoserwal · 2020-03-13T16:00:44Z

jamesnetherton · 2020-03-13T16:19:42Z

Thanks for this 👍

I found some time to refactor the codebase a little and I have a ton of local changes that I'm still testing. I'll try and apply your work on top of it next week and hopefully cut a 0.2.0 release.

jeremyary · 2020-03-17T12:36:12Z

pkg/controller/atlasmap/atlasmap_controller.go

 	return reconcile.Result{}, nil
 }
+
+func (r *ReconcileAtlasMap) getClusterVersionSemVer(err error, reqLogger logr.Logger) (*semver.Version, error, reconcile.Result, bool) {


How does this handle vanilla k8s installations (that aren't OCP)? Check out operator-utils /utils.go/IsOpenShift & LookupOpenShiftVersion and let's evaluate if that tooling is still relevant or can be caught up as there are still a lot of teams using the IsOpenShift check & some using it for versioning as well.

I'm honestly not sure if these features translate back to k8s, but if not, we should probably gate-check that.

In particular, let's look at the bug mentioned here and see if we can clean up the util code and maybe do away with the hard-coded version mapping altogether? Not sure we can, but thought you may want to get familiar with that code and see if it has a home here.

@jeremyary: thanks, I'll check those utils methods and try to validate against k8s as well.

I verified it won't work with K8s. I will replace with utils methods

I reported a breaking change in the operator-sdk testing framework (operator-framework/operator-sdk#2731) which is blocking me to use operator-util methods in the test-cases.

pkg/util/console_link.go

pkg/controller/atlasmap/atlasmap_controller.go

pkg/util/console_link.go

jamesnetherton · 2020-03-23T16:54:28Z

@akoserwal There are some conflicts that need fixing up.

jamesnetherton · 2020-03-24T11:15:54Z

The integration test needs to be a bit smarter as the CI build only runs on OpenShift 3.x at present. So we only want to do assertions on consolelink CRs if the version is 4.x.

akoserwal · 2020-03-24T12:04:00Z

@jamesnetherton: yeah, I am working on it. thanks

akoserwal · 2020-04-01T11:04:35Z

@jamesnetherton: you can review now.

jamesnetherton

Hi @akoserwal. Thanks for your work on this.

I left a few comments.

jamesnetherton · 2020-04-01T12:13:40Z

pkg/controller/atlasmap/action.go

@@ -46,6 +59,7 @@ func newOperatorActions(log logr.Logger, mgr manager.Manager) []action {
 	return []action{
 		newServiceAction(log.WithValues("type", "service"), mgr),
 		routeAction,
+		consoleLink,


I think there's a potential nil pointer exception here. If the operator is not running on OpenShift >= 4.3, then consoleLink will still be appended to the actions slice as nil. When the actions loop runs during reconcile it'll encounter the nil action.

updated code to handle the nil pointer case

jamesnetherton · 2020-04-01T12:16:35Z

pkg/controller/atlasmap/atlasmap_controller.go

@@ -45,7 +58,7 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error {
 		return err
 	}

-	// Watch for changes to primary resource AtlasMap
+		// Watch for changes to primary resource AtlasMap


Unnecessary indentation.

jamesnetherton · 2020-04-01T12:42:15Z

pkg/controller/atlasmap/action_consolelink.go

+				if err != nil {
+					return err
+				} else if err == nil {
+					controllerutil.AddFinalizer(atlasMap, consoleLinkFinalizer)


Just a question, does the removal of the ConsoleLink have to be done from a finalizer?

If you create the ConsoleLink with through action.deployResource(), it'll set a owner reference between the AtlasMap CR and the ConsoleLink.

Console Link (application menu, namespace dashboard) is cluster-scoped. Namespaced dashboard which is part of the console link is namespace scoped. We can not set owner reference for console-link. I am using the finalizer as a flag to allow the deletion of console-link before the Atlasmap cr gets deleted and removes the flag once console-link has been deleted.

Agree on cascading delete not working. However, we ended up doing this without a finalizer, by picking up the CR deletion reconcile call and cleaning it up in the reconcile loop the way we do adds and updates. See kiegroup/kie-cloud-operator#397

I removed the finalizer logic and handle in the reconciler. thank you for the suggestion.

jamesnetherton · 2020-04-02T12:03:59Z

test/e2e/atlasmap_test.go

@@ -113,6 +116,42 @@ func atlasMapDeploymentTest(t *testing.T, f *framework.Framework, ctx *framework
 		}
 		scheme = "https"
 		host = atlasMapRoute.Spec.Host
+
+		//verify Console Link NamespaceDashboard
+		openShiftSemVer := util.GetClusterVersionSemVer(f.KubeConfig)


There's another potential nil pointer exception here. GetClusterVersionSemVer has:

if errors.IsNotFound(err) { // default to OpenShift 3 as ClusterVersion API was introduced in OpenShift 4 openShiftSemVer, _ = semver.NewVersion("3") } else { return nil }

So if an error occurs that is not trapped by the IsNotFound condition, nil is returned and is not handled by the caller. I'm actually seeing this happen on a local Minishift cluster:

$ eval $(minishift docker-env) $ make build $ make install $ make deploy $ oc logs atlasmap-operator-7cdb8b594f-f2t9w {"level":"info","ts":1585821466.9990215,"logger":"cmd","msg":"Go Version: go1.13.5"} {"level":"info","ts":1585821466.9990773,"logger":"cmd","msg":"Go OS/Arch: linux/amd64"} {"level":"info","ts":1585821466.9990823,"logger":"cmd","msg":"Operator SDK Version: v0.15.1"} {"level":"info","ts":1585821466.9990854,"logger":"cmd","msg":"AtlasMap Operator Version: 0.3.0"} {"level":"info","ts":1585821466.9990885,"logger":"cmd","msg":"AtlasMap Operator Git Commit: 3c1271f0"} {"level":"info","ts":1585821466.9990923,"logger":"cmd","msg":"AtlasMap Default Image: docker.io/atlasmap/atlasmap:latest"} {"level":"info","ts":1585821466.9992895,"logger":"leader","msg":"Trying to become the leader."} {"level":"info","ts":1585821467.6745458,"logger":"leader","msg":"Found existing lock with my name. I was likely restarted."} {"level":"info","ts":1585821467.674589,"logger":"leader","msg":"Continuing as the leader."} {"level":"info","ts":1585821468.332753,"logger":"controller-runtime.metrics","msg":"metrics server is starting to listen","addr":"0.0.0.0:8383"} {"level":"info","ts":1585821468.3341796,"logger":"cmd","msg":"Registering Components."} panic: runtime error: invalid memory address or nil pointer dereference [signal SIGSEGV: segmentation violation code=0x1 addr=0x20 pc=0x53b192] goroutine 1 [running]: github.com/Masterminds/semver.(*Version).Prerelease(...) /home/james/go/pkg/mod/github.com/!masterminds/semver@v1.5.0/version.go:146 github.com/Masterminds/semver.constraintGreaterThanEqual(0x0, 0xc000403980, 0xc0001df1a0) /home/james/go/pkg/mod/github.com/!masterminds/semver@v1.5.0/constraints.go:289 +0x22 github.com/Masterminds/semver.(*constraint).check(...) /home/james/go/pkg/mod/github.com/!masterminds/semver@v1.5.0/constraints.go:176 github.com/Masterminds/semver.Constraints.Check(0xc000115380, 0x1, 0x1, 0x0, 0x0) /home/james/go/pkg/mod/github.com/!masterminds/semver@v1.5.0/constraints.go:49 +0x82 github.com/atlasmap/atlasmap-operator/pkg/controller/atlasmap.newOperatorActions(0x18728a0, 0xc0002669c0, 0x188b2e0, 0xc00045e900, 0xc0000b3c20, 0x0, 0x0) atlasmap-operator/pkg/controller/atlasmap/action.go:44 +0x872 github.com/atlasmap/atlasmap-operator/pkg/controller/atlasmap.add(0x188b2e0, 0xc00045e900, 0x182ce00, 0xc000249050, 0x0, 0x0) atlasmap-operator/pkg/controller/atlasmap/atlasmap_controller.go:114 +0x538 github.com/atlasmap/atlasmap-operator/pkg/controller/atlasmap.Add(0x188b2e0, 0xc00045e900, 0xb50504, 0x233a5b0) atlasmap-operator/pkg/controller/atlasmap/atlasmap_controller.go:48 +0x17c github.com/atlasmap/atlasmap-operator/pkg/controller.AddToManager(0x188b2e0, 0xc00045e900, 0x0, 0x0) atlasmap-operator/pkg/controller/controller.go:13 +0x79

jamesnetherton · 2020-04-03T07:02:31Z

@akoserwal I'm happy with the latest changes. Just one final request...

I just created deploy/olm-catalog/atlasmap-operator/0.3.0/atlasmap-operator.v0.3.0.clusterserviceversion.yaml. Please can you add the additional permissions for ConsoleLink.

…cr gets deleted and remove the flag

jamesnetherton force-pushed the master branch from bd71b70 to 8c11a3b Compare March 14, 2020 12:00

jeremyary suggested changes Mar 17, 2020

View reviewed changes

akoserwal force-pushed the feat-consolelink branch from 7d0db9a to b77af0d Compare March 24, 2020 10:16

akoserwal changed the title ~~[WIP] adding consolelink namespacedashboard~~ adding consolelink namespacedashboard Mar 24, 2020

jamesnetherton force-pushed the master branch 2 times, most recently from 6556a07 to 9255586 Compare March 27, 2020 10:23

akoserwal force-pushed the feat-consolelink branch from 81b7375 to c1f6333 Compare March 30, 2020 11:40

jamesnetherton requested changes Apr 1, 2020

View reviewed changes

jamesnetherton reviewed Apr 2, 2020

View reviewed changes

akoserwal added 12 commits April 3, 2020 15:38

adding consolelink namespacedashboard

687026a

fixing the typing error

723a20b

added consolelink implementation as new action type

231fa76

update deployment test for consolelink namespacedashboard

69ff1c0

update consolelink action and removed watcher

e63909c

added fail check for consolev1

de83a98

added finalizer as flag to delete the console-link cr, when atlasmap …

99a1e1a

…cr gets deleted and remove the flag

added util func to check version and add version constraints

d2a72fc

handling nil case for consolelink & fixing unwanted indentation

ef52372

handling nil pointer for the util method getclusterversion

0fc9934

replacing finaziler with reconciler for removing console link

a58e546

adding consolelink permissions to csv

937d769

akoserwal force-pushed the feat-consolelink branch from a87b8e3 to 937d769 Compare April 3, 2020 10:24

jamesnetherton approved these changes Apr 3, 2020

View reviewed changes

jamesnetherton merged commit f4aa863 into atlasmap:master Apr 3, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

adding consolelink namespacedashboard #13

adding consolelink namespacedashboard #13

akoserwal commented Mar 13, 2020

jamesnetherton commented Mar 13, 2020

jeremyary Mar 17, 2020

akoserwal Mar 17, 2020

akoserwal Mar 18, 2020

akoserwal Mar 30, 2020 •

edited

Loading

jamesnetherton commented Mar 23, 2020

jamesnetherton commented Mar 24, 2020

akoserwal commented Mar 24, 2020

akoserwal commented Apr 1, 2020

jamesnetherton left a comment

jamesnetherton Apr 1, 2020

akoserwal Apr 1, 2020

jamesnetherton Apr 1, 2020

akoserwal Apr 1, 2020

jamesnetherton Apr 1, 2020

akoserwal Apr 1, 2020

bmozaffa Apr 1, 2020

akoserwal Apr 2, 2020

jamesnetherton Apr 2, 2020

akoserwal Apr 2, 2020

jamesnetherton commented Apr 3, 2020

adding consolelink namespacedashboard #13

adding consolelink namespacedashboard #13

Conversation

akoserwal commented Mar 13, 2020

jamesnetherton commented Mar 13, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akoserwal Mar 30, 2020 • edited Loading

Choose a reason for hiding this comment

jamesnetherton commented Mar 23, 2020

jamesnetherton commented Mar 24, 2020

akoserwal commented Mar 24, 2020

akoserwal commented Apr 1, 2020

jamesnetherton left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamesnetherton commented Apr 3, 2020

akoserwal Mar 30, 2020 •

edited

Loading