Run verify-codegen.sh on CI #211

asymmetric · 2018-03-14T13:00:21Z

The hack/verify-codegen.sh script checks whether there are any pending codegen changes the committer has forgotten to commit.

Closes #206.

lilic

lgtm overall

lilic · 2018-03-19T08:19:07Z

hack/verify-codegen.sh

+
+if [[ $ret -eq 0 ]]
+then
+  echo "Up to date."


Maybe something like this for more clarity:

Deepcopy generated files up to date.

Just above it says "checking against freshly generated codegen", maybe that's enough?

Nitpick alert! :)

Only reason I said was because currently shows up like this in the Travis logs:

checking against freshly generated codegen
Up to date.

And it doesn't seem connected, maybe just add a : to understand that those two lines of logs belong together.

https://travis-ci.org/habitat-sh/habitat-operator/builds/353421977#L523

lilic · 2018-03-19T08:21:31Z

hack/verify-codegen.sh

+then
+  echo "Up to date."
+else
+  echo "Out of date. Please run hack/update-codegen.sh"


And this something like:

Deepcopy generated files out of date. Please run hack/update-codegen.sh

lilic · 2018-03-19T08:24:39Z

.travis.yml

@@ -25,13 +25,15 @@ before_script:
 - sudo minikube start --vm-driver=none --kubernetes-version=${K8S_VERSION} --extra-config=apiserver.Authorization.Mode=RBAC
 # Fix the kubectl context, as it's often stale.
 - minikube update-context
+# Clone the code generator repo
+- mkdir -p $GOPATH/src/k8s.io && git clone https://github.com/kubernetes/code-generator.git $GOPATH/src/k8s.io/code-generator


Think this would be better to be part of the verify script, so if it that needs to get removed you don't need to chase down the dependencies for it.

Interesting point.. I'll give it a try.

lilic · 2018-03-19T08:26:45Z

.travis.yml

 - make test
- make TESTIMAGE=habitat/habitat-operator:testing e2e
+- make TAG=testing image && make TESTIMAGE=habitat/habitat-operator:testing e2e


Maybe just add image target to e2e instead, so it's run before, as it's needed always anyways.

I thought about this, but I think it's not a good idea: the image target is not built in a way that it knows when it doesn't have to perform any work. Which means that running e2e twice would build the Docker image twice.

Maybe we can refactor the image target to be idempotent, but I didn't think it was in scope for this PR.

I take that back, Docker actually uses the cache, so we can do what you said.

OK, here's the problem: go build is run every time make e2e is run, which doesn't seem right.

In Travis that's not a problem, because we only run the target once, but when I'm testing locally, it is.

Can you explain why doesn't it seem right?

Well it's mainly aimed to run on travis and since we don't do cache there should be fine...

I run it locally :)

Because we have different setups on Travis and locally (running minikube with/without a VM), so some things fail on Travis which work locally - something I found out by runinng e2e tests locally :)

Fair enough! We could also have one target that is make travis as well, if I remember what this discussion was about anyways. :D

lilic · 2018-03-23T13:08:28Z

Is this ready for a re-review?

asymmetric · 2018-04-09T16:09:26Z

`Updated to:

Run on CircleCI
Pin the correct branch for the version of Kubernetes we're targeting, rather than master

asymmetric · 2018-04-10T10:25:01Z

PTALA, test was flaky on CircleCI, hopefully #233 will fix this.

No longer a maintainer.

krnowak

One nit, otherwise LFAD.

krnowak · 2018-04-10T16:09:52Z

hack/verify-codegen.sh

+set -o pipefail
+
+SCRIPT_ROOT=$(dirname "${BASH_SOURCE}")/..
+SCRIPT_BASE=${SCRIPT_ROOT}/../..


This line looks unnecessary - SCRIPT_BASE is not used anywhere.

We perform other operations in the meantime, so the chance that we actually have to wait is lower. Signed-off-by: Lorenzo Manacorda <lorenzo@kinvolk.io>

Signed-off-by: Lorenzo Manacorda <lorenzo@kinvolk.io>

Can be run in a CI. Signed-off-by: Lorenzo Manacorda <lorenzo@kinvolk.io>

Signed-off-by: Lorenzo Manacorda <lorenzo@kinvolk.io>

This allows CI builds to provide feedback earlier, since unit tests don't need the Docker image. Signed-off-by: Lorenzo Manacorda <lorenzo@kinvolk.io>

`GOPATH` on CircleCI is an array, whereas the code-generator script expects it to be a string.

This disables a prompt on our CI setup.

We can't rely on the latest minikube working for us (case in point, the update to 0.26 was breaking the builds).

asymmetric · 2018-04-11T11:12:06Z

Fixed the buld by pinning minikube. The latest version has issues running on both Travis and CircleCI.

krnowak

ShellCheck produces a bunch of warnings for both update-codegen.sh and verify-codegen.sh, but maybe let's fix it in some other PR.

asymmetric · 2018-04-11T12:26:51Z

Sure, feel free to open an issue. I didn't change it much compared to the original, for simplicity's sake.

krnowak · 2018-04-11T12:36:44Z

Filed as #238.

asymmetric requested review from zeenix, krnowak, indradhanush and lilic March 14, 2018 13:00

asymmetric force-pushed the asymmetric/travis-codegen branch from 35e4318 to a833f6d Compare March 14, 2018 16:33

lilic previously requested changes Mar 19, 2018

View reviewed changes

asymmetric mentioned this pull request Mar 28, 2018

Run tests on CircleCI #225

Merged

asymmetric force-pushed the asymmetric/travis-codegen branch 8 times, most recently from d023910 to 5f0001a Compare April 9, 2018 16:05

asymmetric changed the title ~~Run verify-codegen.sh on Travis~~ Run verify-codegen.sh on CI Apr 9, 2018

asymmetric force-pushed the asymmetric/travis-codegen branch from 5f0001a to 4118f6a Compare April 9, 2018 16:23

asymmetric removed the request for review from zeenix April 10, 2018 10:25

krnowak suggested changes Apr 10, 2018

View reviewed changes

Lorenzo Manacorda and others added 7 commits April 11, 2018 11:12

Move wait for Kubernetes as last step

4b53fc1

We perform other operations in the meantime, so the chance that we actually have to wait is lower. Signed-off-by: Lorenzo Manacorda <lorenzo@kinvolk.io>

Clone code-generator repo in Travis

638ea5e

Signed-off-by: Lorenzo Manacorda <lorenzo@kinvolk.io>

Add hack/verify-codegen.sh script

43ec99f

Can be run in a CI. Signed-off-by: Lorenzo Manacorda <lorenzo@kinvolk.io>

Run verify-codegen.sh in Travis

a5624d3

Signed-off-by: Lorenzo Manacorda <lorenzo@kinvolk.io>

Only build image when needed

27ac3b3

This allows CI builds to provide feedback earlier, since unit tests don't need the Docker image. Signed-off-by: Lorenzo Manacorda <lorenzo@kinvolk.io>

Prettify hack/verify-codegen.sh

621c7ae

circleci: Run code-gen script

ebe40d3

asymmetric added 3 commits April 11, 2018 11:12

circleci: Hardcode GOPATH

6cae8f7

`GOPATH` on CircleCI is an array, whereas the code-generator script expects it to be a string.

*: Run code-generator at branch release-1.9

13e39d0

travis: Clone code-generator at branch release-1.9

fe0f781

asymmetric force-pushed the asymmetric/travis-codegen branch from 4118f6a to fe0f781 Compare April 11, 2018 09:13

asymmetric added 2 commits April 11, 2018 12:31

ci: set WarnReportErrorPrompt to false

f6814ea

This disables a prompt on our CI setup.

ci: Print minikube logs on failure

fa5082e

asymmetric force-pushed the asymmetric/travis-codegen branch from fb2a3dc to 82de1fa Compare April 11, 2018 10:46

ci: Pin minikube to 0.25.2

3f735d3

We can't rely on the latest minikube working for us (case in point, the update to 0.26 was breaking the builds).

asymmetric force-pushed the asymmetric/travis-codegen branch from 82de1fa to 3f735d3 Compare April 11, 2018 10:52

krnowak approved these changes Apr 11, 2018

View reviewed changes

asymmetric merged commit 1708b63 into habitat-sh:master Apr 11, 2018

asymmetric deleted the asymmetric/travis-codegen branch April 11, 2018 12:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Run verify-codegen.sh on CI #211

Run verify-codegen.sh on CI #211

asymmetric commented Mar 14, 2018 •

edited

Loading

lilic left a comment

lilic Mar 19, 2018

asymmetric Mar 19, 2018

lilic Mar 19, 2018

lilic Mar 19, 2018

lilic Mar 19, 2018

asymmetric Mar 19, 2018

lilic Mar 19, 2018

asymmetric Mar 19, 2018

asymmetric Mar 19, 2018

asymmetric Mar 19, 2018 •

edited

Loading

lilic Mar 19, 2018

lilic Mar 19, 2018

asymmetric Mar 19, 2018

lilic Mar 19, 2018

asymmetric Mar 19, 2018 •

edited

Loading

lilic Mar 19, 2018

lilic commented Mar 23, 2018

asymmetric commented Apr 9, 2018 •

edited

Loading

asymmetric commented Apr 10, 2018

krnowak left a comment

krnowak Apr 10, 2018

asymmetric commented Apr 11, 2018 •

edited

Loading

krnowak left a comment

asymmetric commented Apr 11, 2018

krnowak commented Apr 11, 2018

Run verify-codegen.sh on CI #211

Run verify-codegen.sh on CI #211

Conversation

asymmetric commented Mar 14, 2018 • edited Loading

lilic 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

asymmetric Mar 19, 2018 • edited Loading

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

asymmetric Mar 19, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lilic commented Mar 23, 2018

asymmetric commented Apr 9, 2018 • edited Loading

asymmetric commented Apr 10, 2018

krnowak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asymmetric commented Apr 11, 2018 • edited Loading

krnowak left a comment

Choose a reason for hiding this comment

asymmetric commented Apr 11, 2018

krnowak commented Apr 11, 2018

asymmetric commented Mar 14, 2018 •

edited

Loading

asymmetric Mar 19, 2018 •

edited

Loading

asymmetric Mar 19, 2018 •

edited

Loading

asymmetric commented Apr 9, 2018 •

edited

Loading

asymmetric commented Apr 11, 2018 •

edited

Loading