AEROGEAR-9985 - Upgrade mss-operator to operator-sdk v0.10.0 #177
Conversation
@grdryn @psturc @laurafitzgerald |
@@ -7,7 +7,9 @@ jobs: | |||
working_directory: /go/src/github.com/aerogear/mobile-security-service-operator | |||
|
|||
docker: | |||
- image: circleci/golang:1.10 | |||
- image: circleci/golang:1.12 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@psturc Do you know what version of Go is installed in the Jenkins agent that compiles?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the version of go in Jenkins slave is v1.12.6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, should be no problems then!
@@ -216,13 +214,13 @@ code/gen: | |||
.PHONY: test/run | |||
test/run: | |||
@echo Running tests: | |||
GOCACHE=off go test -cover $(TEST_PKGS) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What was the significance of this being here originally, and is it good that it's being removed now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From Go 1.1.2 release notes (https://golang.org/doc/go1.12)
The build cache is now required as a step toward eliminating $GOPATH/pkg. Setting the environment variable GOCACHE=off will cause go commands that write to the cache to fail.
I had to remove this parameter to make things working with Go 1.12
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. @grdryn are you doing the verification? If you want me to do it just ping me here.
I haven't done any verification yet, I will though 👍 |
@aliok along with the other comments I left in my original review, could you also add a stage like the following to the Jenkinsfile, so that we're using the correct version of the operator-sdk CLI there: https://github.com/aerogear/unifiedpush-operator/blob/master/Jenkinsfile#L61..L73 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have verified this now. I believe at least my previous review comment about wrapping the creation of the servicemonitor should be addressed, since otherwise local development is a little broken.
Co-Authored-By: Gerard Ryan <gryan@redhat.com>
All comments addressed now. |
Verification instructions:
Enable anyuid:
Install Integreatly application monitoring operator:
If you see an error in Prometheus regarding file system permissions, you need to
chmod 777
the host path:Deploy MSS operator, monitoring for it, and an example app
Check these:
Note that some of the stuff in Prom/Grafana doesn't show up properly in Minishift. I assume they will show up in RHMI cluster as these Prom queries came all the way until today.