Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

vagrant: Stop provisioning VM if one step fails #10430

Merged
merged 1 commit into from Mar 4, 2020
Merged

vagrant: Stop provisioning VM if one step fails #10430

merged 1 commit into from Mar 4, 2020

Conversation

pchaigno
Copy link
Member

@pchaigno pchaigno commented Mar 3, 2020 •
edited by tgraf

Currently, if for example the build provisioning step of the development Vagrant VM fails, vagrant continues with subsequent steps and ultimately fails to install and start cilium. This pull request updates the Vagrantfile and associated scripts to fail early.

This change is Reviewable

@pchaigno pchaigno added pending-review area/misc Impacts miscellaneous areas of the code not otherwise owned by another area. release-note/misc This PR makes changes that have no direct user impact. labels Mar 3, 2020
@pchaigno pchaigno requested a review from a team as a code owner March 3, 2020 12:14
@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.8.0 Mar 3, 2020
@coveralls
Copy link

coveralls commented Mar 3, 2020 •
edited

Coverage Status

Coverage remained the same at 45.642% when pulling e43287e on pr/pchaigno/fail-early-vagrant-provisioning into fe98c5b on master.

@errordeveloper
Copy link
Contributor

errordeveloper commented Mar 3, 2020 •
edited

How about using more explicit option names, and setting pipefail as well as nounset?

set -o errexit
set -o pipefail
set -o nounset

@pchaigno pchaigno force-pushed the pr/pchaigno/fail-early-vagrant-provisioning branch from 5db9697 to 5af12d1 Compare March 3, 2020 15:18
@pchaigno
vagrant: Stop provisioning VM if one step fails
e43287e 
Currently, if e.g., the build provisioning step of the development
Vagrant VM fails, vagrant continues with subsequent steps and ultimately
fails to install and start cilium. This commit updates the Vagrantfile
and associated scripts to fail early.

Signed-off-by: Paul Chaignon <paul@cilium.io>
@pchaigno pchaigno force-pushed the pr/pchaigno/fail-early-vagrant-provisioning branch from 5af12d1 to e43287e Compare March 3, 2020 15:20
@pchaigno
Copy link
Member Author

pchaigno commented Mar 3, 2020

How about using more explicit option names, and setting pipefail as well as nounset?

Sure 👍

I tested this change on all three dev. VM configs (default, K8S=1, and K8S=1 NWORKERS=1). The only fix needed was for the K8S variable in contrib/vagrant/start.sh.

@pchaigno
Copy link
Member Author

pchaigno commented Mar 3, 2020

test-me-please

1 similar comment
@aanm
Copy link
Member

aanm commented Mar 4, 2020

test-me-please

@aanm aanm merged commit e35eeba into master Mar 4, 2020
1.8.0 automation moved this from In progress to Merged Mar 4, 2020
@aanm aanm deleted the pr/pchaigno/fail-early-vagrant-provisioning branch March 4, 2020 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aanm aanm aanm approved these changes

Assignees
No one assigned
Labels
area/misc Impacts miscellaneous areas of the code not otherwise owned by another area. release-note/misc This PR makes changes that have no direct user impact.
Projects
No open projects
1.8.0
  
Merged
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@pchaigno @coveralls @errordeveloper @aanm