scylladb / scylla-ansible-roles

Ansible roles for deploying and managing Scylla, Scylla-Manager and Scylla-Monitoring
44 stars 38 forks source link

Proper way of updating APT cache #44

Closed elcomtik closed 2 years ago

elcomtik commented 3 years ago

I just discovered your roles and found an issue at https://github.com/scylladb/scylla-ansible-roles/blob/master/ansible-scylla-node/tasks/Debian.yml#L105-L109

This doesn't work, it will not update the cache and the next task will fail.

The update_cache parameter should be used instead https://docs.ansible.com/ansible/latest/collections/ansible/builtin/apt_module.html

It may be placed in this task or directly in a task which installs packages.

dyasny commented 3 years ago

I tested this on Ubuntu 18 and 20 as well as Debian 9 and it worked. But frankly, I've done MUCH more testing around CentOS as the primary distribution, so things might have slipped through the cracks and what worked at some point could have stopped working. Without a CI and nightly builds in place, this is hard to track.

elcomtik commented 3 years ago

Few lines above is a similar task, which has it OK. This one you probably missed. Of course, each code without tests is broken by design (my favorite quote). Did you consider using the molecule test suite? I have some experience with it and it solves at least some basic problems such as this one without much effort.

dyasny commented 3 years ago

Molecule was on my TODO actually, but I am not the current maintainer of this project, I just try to help when I can (this is also why I don't have the machines to test changes on).

In any case, if you find it useful and you have the time, I'm sure @tarzanek or someone from his team will be happy to merge a PR

elcomtik commented 3 years ago

@dyasny I discussed tests with @tarzanek on ScyllaDB-Users slack. It would be best if this would get integrated with some CI/CD platform, I may provide PR for molecule tests, Travis/GitLab/GitHub, and release through ansible-galaxy. There are some decisions about tooling used on your side, which must be done before I would contribute.

dyasny commented 3 years ago

@elcomtik I agree, a CI/CD pipeline was on the roadmap, but I'm not sure where it is right now

elcomtik commented 3 years ago

I've created example tests and github action workflow for ansible-scylla-node role https://github.com/scylladb/scylla-ansible-roles/pull/57.