Update `kitchen.yml` to use pre-salted images

dafyddj commented 5 years ago

Closes #34

myii commented 5 years ago

@dafyddj Thanks for running with this, look forward to seeing how you get along! Feel free to ping me at any point during this process, either here or in Slack, if there's any specific issues along the way.

myii commented 5 years ago

@dafyddj We're in the process of upgrading the matrix to remove EOL platforms. If possible, please use these two updated files as a basis:

myii commented 5 years ago

Collecting some notes from the discussion in Slack earlier. This one about using gid: vault when replacing the recently deprecated gid_from_name.

- - -

20:59 myii @dafydd We may need to consider https://github.com/saltstack-formulas/users-formula/issues/198#issuecomment-494927619. Or will it always be gid: vault?

21:00 myii I haven't looked into this in any detail, so it may not work in this situation. If it is useful, then a simple Jinja if block based on the saltversioninfo grain will probably be sufficient.

22:52 dafydd my thinking here was that a jinja if would not particularly promote readability - when the change i propose works in all current and dev versions

22:53 myii Sure, so it will always be gid: vault, never anything else?

22:56 dafydd the aim is that the user and the group have the same name, and the user's default group is the group of the same name. so as things stand, yes it will never be anything else

22:56 dafydd one could argue that the user/group name should be configurable but currently the formula follows the upstream (vault) install guidelines

22:57 myii Looking at https://github.com/saltstack-formulas/vault-formula/blob/6eddca53bff0d874406bde23014e0f829ae59995/vault/package/install.sls, there are three places that vault is used for user/group config. How about putting this in defaults.yaml and using it from the map instead?

22:58 myii This then allows for the future possibility of overriding via. the config/pillar. That doesn't need to be mentioned in pillar.example at the moment, though.

22:59 myii 4, if you consider - home: /var/lib/vault as well.

22:59 dafydd i'd like to keep the PR focused on the kitchen/travis changes - by all means we can add an issue for this stuff for further work

23:00 myii That's fine, the literal use of vault matches the rest of the .sls. And avoids the Jinja if.

-	-	-
20:59	myii	@dafydd We may need to consider https://github.com/saltstack-formulas/users-formula/issues/198#issuecomment-494927619. Or will it always be `gid: vault`?
21:00	myii	I haven't looked into this in any detail, so it may not work in this situation. If it is useful, then a simple Jinja `if` block based on the `saltversioninfo` grain will probably be sufficient.
22:52	dafydd	my thinking here was that a jinja `if` would not particularly promote readability - when the change i propose works in all current and dev versions
22:53	myii	Sure, so it will always be `gid: vault`, never anything else?
22:56	dafydd	the aim is that the user and the group have the same name, and the user's default group is the group of the same name. so as things stand, yes it will never be anything else
22:56	dafydd	one could argue that the user/group name should be configurable but currently the formula follows the upstream (vault) install guidelines
22:57	myii	Looking at https://github.com/saltstack-formulas/vault-formula/blob/6eddca53bff0d874406bde23014e0f829ae59995/vault/package/install.sls, there are three places that `vault` is used for user/group config. How about putting this in `defaults.yaml` and using it from the map instead?
22:58	myii	This then allows for the future possibility of overriding via. the config/pillar. That doesn't need to be mentioned in `pillar.example` at the moment, though.
22:59	myii	4, if you consider `- home: /var/lib/vault` as well.
22:59	dafydd	i'd like to keep the PR focused on the kitchen/travis changes - by all means we can add an issue for this stuff for further work
23:00	myii	That's fine, the literal use of `vault` matches the rest of the `.sls`. And avoids the Jinja `if`.

dafyddj commented 5 years ago

@myii Changes made.

myii commented 5 years ago

Thanks @dafyddj, this has been merged.

Just noticed that we're using kitchen test instead of kitchen verify in .travis.yml, introduced by commit c026d46 in #22. I know what it does but I'm not sure what benefit it brings here. The objective is to ensure consistency across formulas, so I was about to request a change to verify but I thought I'd ask you first. If test is superior, we can make that change in the template-formula and propagate from there.

saltstack-formulas-travis commented 5 years ago

:tada: This PR is included in version 1.1.0 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket:

dafyddj commented 5 years ago

Using test becomes more important when you have multiple test suites, because the suites run consecutively on the same Travis instance (debian-9-develop-py3 or ubuntu-1804-2019-2-py3 etc.) (And if they didn't use the same instance, that would be a massive time sink) Using verify essentially leaves several docker containers running on the same instance, and when I worked on #22 I think I came up against some resource limits on Travis. But that was many months ago, and perhaps those resource limits or problems were spurious - I haven't revisited it. However, for me test seems generally "cleaner" - you ensure a clean slate before and after the testing - and seems to be the usage that the test command is designed for.

In fact, I've just looked at the docs and they seem to agree: In general, use the test subcommand to verify the end-to-end quality of a cookbook. Use the converge and verify subcommands during the normal the day-to-day development of a cookbook.

myii commented 5 years ago

@javierbertoli @daks Can I bring you into the discussion here, based on the two comments above?

@dafyddj Using kitchen test locally makes sense, you usually don't want the containers hanging around. With Travis, I'd be surprised if the containers aren't destroyed automatically; implicitly adding the destroy adds to the test run, as seen here. But if there is some benefit when using multiple test suites, then we really should cater for that.

dafyddj commented 5 years ago

With Travis, I'd be surprised if the containers aren't destroyed automatically

After a full test run, sure; I'm talking about intra-suite, if that makes my thinking clearer.

javierbertoli commented 5 years ago

As far as I understand it, the main differences are:

kitchen test will perform a full cycle, as described in a kitchen --help command: # Test (destroy, create, converge, setup, verify and destroy) one or more instances
kitchen verify will just run the verifier.

But the key thing is that any kitchen command will run all the previous steps required to run it. Ie,

kitchen converge will also run a kitchen create if the instance does not exist
kitchen verify will also run kitchen create and kitchen converge if the instance does not exist.

So in the end, if no instance is running, verify will perform the same steps as test.

The difference, when running locally, is that kitchen test will run the whole cycle and destroy the instance when tests passes while verify won't. So, if you want to test a new change, all the steps will be run again. That's why, when you run locally, you usually run the steps individually:

kitchen verify (will run all the steps but won't destroy the instace)
modify your code
kitchen converge (to run the new code)
kitchen verify (to verify the changes)
kitchen destroy (when you finish your work and don't want to keep the instance around anymore)

Regarding re-use across suites, I don't think it will be useful, unless suites share some docker layer.

myii commented 5 years ago

@dafyddj Just working on getting the new pre-salted images into the template-formula, including amazonlinux-2, which uses systemd. In terms of https://github.com/saltstack-formulas/vault-formula/pull/35#discussion_r301994954, how about we use centos-6 instead, since that at least uses /sbin/init?

daks commented 5 years ago

@myii as said in slack

Hi kitchen test is equivalent to kitchen create + kitchen converge + kitchen verify + kitchen destroy Those 4 steps are "independent" but when you run one, if previous are not done, it run them, that's why when we run verify it creates and converge But yes, in reality kitchen test is better because it destroys the instance at the end. Kitchen converge/verify are great for dev, when you don't want/need to recreate from scratch your instance everytime (edited) So now I think about it I agree with @dafyddj about using test instead of verify

saltstack-formulas / vault-formula

Update `kitchen.yml` to use pre-salted images #35