[#115845521] Move bosh-cli to Makefile #165

combor · 2016-03-17T15:51:59Z

What

This is a fix for bosh-cli.sh as #159 introduced SYSTEM_DNS_ZONE_NAME variable. This var is set only by running make so we had to move bosh-cli.sh execution to Makefile. Additionally we re-arranged all targets so it's now possible to execute make $ENV bootstrap. Read the updated README if you want more details.

How to review

Check if make dev bosh-cli works properly and connects to Concourse container. Also, run create-bosh-cloudfoundry pipeline and make sure that test-bosh-cli job succeeds .

Who can review

Not @jimconner or @combor

dcarley · 2016-03-17T15:56:44Z

Makefile

+prod: check-env-vars set_env_class_prod ## Set Envirnoment to Production 
+
+.PHONY: bootstrap
+bootstrap: vagrant-deploy ## Start bootsrap 


We don't need a separate target for vagrant-deploy anymore. The contents could be moved into this target like bootstrap-destroy.

Correct. Fixed.

mtekel · 2016-03-17T16:19:28Z

bosh-cli-test is nice, but does it really need to run on every build (it's being triggered)?

mtekel · 2016-03-17T16:21:36Z

Also, given the amount of tests we have now, would it make sense to create a group for tests only?

dcarley · 2016-03-17T16:23:48Z

bosh-cli-test is nice, but does it really need to run on every build (it's being triggered)?

Would we know if it was broken if we didn't trigger it on every build?

mtekel · 2016-03-17T16:25:56Z

Maybe we can just trigger as apart of review process?

combor · 2016-03-17T16:28:24Z

Also, given the amount of tests we have now, would it make sense to create a group for tests only?

Done.

dcarley · 2016-03-17T16:29:48Z

Maybe we can just trigger as apart of review process?

Nah, we should leave it automated. Otherwise we'll forget to run it and then there's no benefit. It shouldn't cause any harm running it each time - I believe it's in parallel to the other tests, so it won't slow anything down.

mtekel · 2016-03-17T17:29:03Z

concourse/scripts/bosh-cli.sh

@@ -25,4 +25,4 @@ $FLY_CMD -t "${FLY_TARGET}" \
  intercept \
  --build="${BUILD_NUMBER}"\
  --step=one-off \
-  ${2:-sh}
+  "${2:-sh}"


squash this in to previous commit

mtekel · 2016-03-17T17:35:53Z

Would it be a better test to do some actual operation inside bosh-cli container, like bosh vms? Currently we just check if we can run that container at all... (and if it has true command).

mtekel · 2016-03-17T17:44:29Z

Apart from these last remarks it all works as described.

alext · 2016-03-18T09:14:54Z

Would it be a better test to do some actual operation inside bosh-cli container, like bosh vms?

I'm not sure that's necessary. This is intended to verify that the container orchestration works. to get this far it will have had to do a bosh login, so we've already verified that it can talk to bosh etc.

alext · 2016-03-18T09:16:44Z

concourse/pipelines/create-bosh-cloudfoundry.yml

+              export CONCOURSE_ATC_PASSWORD
+              ./paas-cf/concourse/scripts/bosh-cli.sh {{deploy_env}} true
+
+
  - name: tag-release


Do we want to trigger this from the new test task as well?

combor · 2016-03-18T09:50:03Z

All comments resolved. Please re-check.

mtekel · 2016-03-18T10:32:29Z

Few things about bosh-cli test:

the commit message is wrong (contains description of curl-ssl)
what exactly is this trying to test? If the test wants to test that we (operators) can still get interactive and working bosh shell, then it should do the same steps as we do - "make dev bosh-cli" - and run a 'bosh vms' or 'bosh cck' command, to verify that the code of the (1) makefile, (2) bosh-cli script and (3) bosh-cli.yml are all working.
If it intends to test only a single aspect of the chain (ability to do fly hijack), then it should be renamed (e.g. test fly-intercept) and we should think about testing other parts of the bosh-cli chain

combor · 2016-03-18T11:25:36Z

the commit message is wrong (contains description of curl-ssl)

Fixed.

alext · 2016-03-18T11:27:18Z

what exactly is this trying to test?

Following a discussion, I think it's reasonable to extend the test to call bosh status. This actually verifies that it can talk to bosh instead of the implicit reliance on the bosh login having happened.

We have added new bosh-cli target and simplified existing bootstrap targets. It's now possible to execute `make dev bosh-cli` and `make dev bootstrap`.

combor · 2016-03-18T16:37:44Z

I believe that all comments have been resolved.

alext · 2016-03-18T16:50:58Z

README.md

+necessary to start with a comletely new deployment.
+
+**Note:** Different AWS accounts use different DNS names, so it'll be necessary
+to adjust some of the instructions above accordingly.


I don't think you meant to add this back in. These docs have moved into the separate linked doc.

Probably it's a rebase leftover. I'll remove it.

We need small adjustments of README after re-arranging Makefile targets.

So we can pass custom command to run inside container.

This is a new job that is testing whether bosh-cli script is not broken. It uses ruby container and installs bash as all scripts need it. Next it connects to Concourse container and exits.

mtekel · 2016-03-21T14:42:58Z

works, merging

[#115845521] Move bosh-cli to Makefile

keymon · 2016-03-22T11:12:57Z

Now self-update-pipelines does not work!!! 😱

It only calls make dev or whatever.

After #165 you must pass the target `pipelines` to update the pipelines, but the `self-update-pipeline.sh` script was not updated.

keymon · 2016-03-22T11:17:01Z

PR to fix self-update #172

After #165 you must pass the target `pipelines` to update the pipelines, but the `self-update-pipeline.sh` script was not updated.

Full diff: alphagov/paas-admin@v0.82.0...v0.94.0 Contains the following PRs by human beings: - #146 Stub APIs needed for the calculator - #148 Request logging - #160 Use javascript naming for request log interceptor - #173 Support: Fix bug showing allocations - #180 Skip calculator getQuote if there are no items - #181 Remove low-value, flakey test And the following dependabot specials: - #161 Bump nunjucks from 3.1.3 to 3.2.0 - #165 Bump glob from 7.1.2 to 7.1.3 - #164 Bump @types/source-map-support from 0.4.1 to 0.5.0 - #162 Bump webpack-cli from 3.1.0 to 3.3.1 - #170 Bump pino from 5.1.0 to 5.12.3 - #169 Bump @types/jest from 23.3.1 to 24.0.11 - #168 Bump nodemon-webpack-plugin from 4.0.3 to 4.0.8 - #167 Bump express-static-gzip from 1.1.1 to 1.1.3 - #166 Bump @types/uuid from 3.4.3 to 3.4.4 - #163 Bump @types/helmet from 0.0.38 to 0.0.43 - #174 Bump tslint-microsoft-contrib from 5.2.0 to 6.1.1 - #175 Bump qs from 6.5.2 to 6.7.0 - #176 Bump moment from 2.22.2 to 2.24.0 - #177 Bump jest from 23.5.0 to 23.6.0 - #178 Bump @types/passport from 0.4.6 to 1.0.0 - #179 Bump jest and ts-jest

combor changed the title ~~115845521 fix bosh cli~~ [#115845521] Move bosh-cli to Makefile Mar 17, 2016

mtekel self-assigned this Mar 17, 2016

dcarley reviewed Mar 17, 2016
View reviewed changes

combor force-pushed the 115845521_fix_bosh_cli branch from 101be1b to 6c7d50c Compare March 17, 2016 16:17

combor force-pushed the 115845521_fix_bosh_cli branch from 6c7d50c to 80d8354 Compare March 17, 2016 16:21

combor force-pushed the 115845521_fix_bosh_cli branch from 80d8354 to 9d9bb42 Compare March 17, 2016 16:27

jimconner force-pushed the 115845521_fix_bosh_cli branch from 4dbb4e0 to 3724a6f Compare March 17, 2016 16:44

mtekel reviewed Mar 17, 2016
View reviewed changes

alext reviewed Mar 18, 2016
View reviewed changes

combor force-pushed the 115845521_fix_bosh_cli branch from 3724a6f to 73a7f16 Compare March 18, 2016 09:49

combor force-pushed the 115845521_fix_bosh_cli branch from 73a7f16 to 07c1d57 Compare March 18, 2016 11:25

combor force-pushed the 115845521_fix_bosh_cli branch 2 times, most recently from 4360bf3 to 37ac77e Compare March 18, 2016 15:38

combor force-pushed the 115845521_fix_bosh_cli branch 3 times, most recently from 64c4fa7 to 4b5d3b1 Compare March 18, 2016 16:21

Introduce bosh-cli target

df50068

We have added new bosh-cli target and simplified existing bootstrap targets. It's now possible to execute `make dev bosh-cli` and `make dev bootstrap`.

combor force-pushed the 115845521_fix_bosh_cli branch from 4b5d3b1 to fde728c Compare March 18, 2016 16:36

alext reviewed Mar 18, 2016
View reviewed changes

Piotr Komborski added 3 commits March 21, 2016 09:52

Update README to describe new bosh-cli target

3f6b12e

We need small adjustments of README after re-arranging Makefile targets.

Parametrize bosh-cli

dd4c061

So we can pass custom command to run inside container.

Add test-bosh-cli job

d0678ec

This is a new job that is testing whether bosh-cli script is not broken. It uses ruby container and installs bash as all scripts need it. Next it connects to Concourse container and exits.

combor force-pushed the 115845521_fix_bosh_cli branch from fde728c to d0678ec Compare March 21, 2016 09:53

mtekel added a commit that referenced this pull request Mar 21, 2016

Merge pull request #165 from alphagov/115845521_fix_bosh_cli

2fef897

[#115845521] Move bosh-cli to Makefile

mtekel merged commit 2fef897 into master Mar 21, 2016

mtekel deleted the 115845521_fix_bosh_cli branch March 21, 2016 14:43

keymon added a commit that referenced this pull request Mar 22, 2016

Add target pipelines to self-update-pipelines.sh

0c72922

After #165 you must pass the target `pipelines` to update the pipelines, but the `self-update-pipeline.sh` script was not updated.

keymon mentioned this pull request Mar 22, 2016

[#115845521] Add target pipelines to self-update-pipelines.sh #172

Merged

keymon added a commit that referenced this pull request Mar 22, 2016

Add target pipelines to self-update-pipelines.sh

21483f1

After #165 you must pass the target `pipelines` to update the pipelines, but the `self-update-pipeline.sh` script was not updated.

keymon added a commit that referenced this pull request Mar 23, 2016

Add target pipelines to self-update-pipelines.sh

9e7d848

After #165 you must pass the target `pipelines` to update the pipelines, but the `self-update-pipeline.sh` script was not updated.

keymon added a commit that referenced this pull request Mar 23, 2016

Add target pipelines to self-update-pipelines.sh

dd93327

After #165 you must pass the target `pipelines` to update the pipelines, but the `self-update-pipeline.sh` script was not updated.

keymon added a commit that referenced this pull request Mar 23, 2016

Add target pipelines to self-update-pipelines.sh

06b1342

After #165 you must pass the target `pipelines` to update the pipelines, but the `self-update-pipeline.sh` script was not updated.

keymon added a commit that referenced this pull request Mar 23, 2016

Add target pipelines to self-update-pipelines.sh

852b6c2

After #165 you must pass the target `pipelines` to update the pipelines, but the `self-update-pipeline.sh` script was not updated.

keymon added a commit that referenced this pull request Mar 23, 2016

Add target pipelines to self-update-pipelines.sh

639670d

After #165 you must pass the target `pipelines` to update the pipelines, but the `self-update-pipeline.sh` script was not updated.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#115845521] Move bosh-cli to Makefile #165

[#115845521] Move bosh-cli to Makefile #165

combor commented Mar 17, 2016

dcarley Mar 17, 2016

combor Mar 17, 2016

mtekel commented Mar 17, 2016

mtekel commented Mar 17, 2016

dcarley commented Mar 17, 2016

mtekel commented Mar 17, 2016

combor commented Mar 17, 2016

dcarley commented Mar 17, 2016

mtekel Mar 17, 2016

mtekel commented Mar 17, 2016

mtekel commented Mar 17, 2016

alext commented Mar 18, 2016

alext Mar 18, 2016

combor commented Mar 18, 2016

mtekel commented Mar 18, 2016

combor commented Mar 18, 2016

alext commented Mar 18, 2016

combor commented Mar 18, 2016

alext Mar 18, 2016

combor Mar 21, 2016

combor Mar 21, 2016

mtekel commented Mar 21, 2016

keymon commented Mar 22, 2016

keymon commented Mar 22, 2016

[#115845521] Move bosh-cli to Makefile #165

[#115845521] Move bosh-cli to Makefile #165

Conversation

combor commented Mar 17, 2016

What

How to review

Who can review

dcarley Mar 17, 2016

Choose a reason for hiding this comment

combor Mar 17, 2016

Choose a reason for hiding this comment

mtekel commented Mar 17, 2016

mtekel commented Mar 17, 2016

dcarley commented Mar 17, 2016

mtekel commented Mar 17, 2016

combor commented Mar 17, 2016

dcarley commented Mar 17, 2016

mtekel Mar 17, 2016

Choose a reason for hiding this comment

mtekel commented Mar 17, 2016

mtekel commented Mar 17, 2016

alext commented Mar 18, 2016

alext Mar 18, 2016

Choose a reason for hiding this comment

combor commented Mar 18, 2016

mtekel commented Mar 18, 2016

combor commented Mar 18, 2016

alext commented Mar 18, 2016

combor commented Mar 18, 2016

alext Mar 18, 2016

Choose a reason for hiding this comment

combor Mar 21, 2016

Choose a reason for hiding this comment

combor Mar 21, 2016

Choose a reason for hiding this comment

mtekel commented Mar 21, 2016

keymon commented Mar 22, 2016

keymon commented Mar 22, 2016