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.

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

docker Chef 18 compatibility #1222

Closed
djessich opened this issue Dec 1, 2022 · 7 comments
Closed

docker Chef 18 compatibility #1222

djessich opened this issue Dec 1, 2022 · 7 comments

Comments

@djessich
Copy link

djessich commented Dec 1, 2022

👻 Brief Description

When executing docker_service resource (maybe others from libraries folder too?) from this cookbook, its configured values in resource properties are ignored when running with Chef 18.

🥞 Cookbook version

10.2.4

👩‍🍳 Chef-Infra Version

18.0.185

🎩 Platform details

Docker Container running with Test Kitchen (e.g. RHEL 8)

Steps To Reproduce

Run

docker_service 'default' do
  install_method 'package'
  setup_docker_repo false
  group 'someothergroup' # <-- does not work
  action [:create, :start]
end

will render in /etc/systemd/system/docker.service

# TRUNCATED OUTPUT
ExecStart=/usr/bin/dockerd  --group=docker --pidfile=/var/run/docker.pid --containerd=/run/containerd/containerd.sock
# TRUNCATED OUTPUT

🚓 Expected behavior

Should render in /etc/systemd/system/docker.service

# TRUNCATED OUTPUT
ExecStart=/usr/bin/dockerd  --group=someothergroup --pidfile=/var/run/docker.pid --containerd=/run/containerd/containerd.sock
# TRUNCATED OUTPUT

Pls mind --group=someothergroup as configured by resource properties.

➕ Additional context

Works fine with Chef 17.

Maybe somehow related to unified_mode that became enabled with Chef 18 by default?

@djessich
Copy link
Author

djessich commented Dec 1, 2022

After searching around the issue its definitely a problem with unified_mode. By setting DockerCookbook::DockerService.unified_mode(false) from within my wrapper cookbook, the values are correctly set, so my example above works like a charm.

@Stromweld
Copy link
Contributor

If you can submit a PR for that we'd happily review it and get it released.

@djessich
Copy link
Author

djessich commented Dec 1, 2022

@Stromweld Unfortunately, its not that easy to implement "correctly". Well you may be able to add unified_mode false to the custom resources defined in libraries but this makes it incompatible with unified mode. I think it is better to refactor all resources in libraries folder to custom resources, but this requires very deep knowledge in functional requirements for this cookbook which I do not have.

ramereth added a commit that referenced this issue Feb 2, 2023
Fixing this cookbook for 18 is going to require a lot of refactoring in the
resources.

Signed-off-by: Lance Albertson <lance@osuosl.org>
ramereth added a commit that referenced this issue Feb 3, 2023
Fixing this cookbook for 18 is going to require a lot of refactoring in the
resources.

Signed-off-by: Lance Albertson <lance@osuosl.org>
ramereth added a commit that referenced this issue Feb 3, 2023
Fixing this cookbook for 18 is going to require a lot of refactoring in the
resources.

Signed-off-by: Lance Albertson <lance@osuosl.org>
ramereth added a commit that referenced this issue Feb 3, 2023
* Fix various CI issues

- Fix support for running on cgroupv2
- Split out amazonlinux-2 to Ubuntu 20.04 runner

Signed-off-by: Lance Albertson <lance@osuosl.org>

* Run smoke tests on VMs

Also update tests with the new version of docker that's out.

Signed-off-by: Lance Albertson <lance@osuosl.org>

* Set a ceiling of Chef 17 as Chef 18 is broken due to #1222

Fixing this cookbook for 18 is going to require a lot of refactoring in the
resources.

Signed-off-by: Lance Albertson <lance@osuosl.org>

---------

Signed-off-by: Lance Albertson <lance@osuosl.org>
@b-dean
Copy link
Contributor

b-dean commented Apr 19, 2023

@djessich, I may be misunderstanding Chef's documentation on unified_mode, but it seems like the change in Chef 18 is that unified_mode is true by default. Not true always, it still is something resources can set to false. But by default it is true in Chef 18. It's not that it can only work in unified mode. Seems like it is still valid to set unified_mode false for resources that cannot use it.

In fact, I think that's why they're even saying something, so people will know, "hey this is the new default, so fix your stuff" either by making it work with unified mode, or specifically disabling unified mode for the resources.

@ramereth
Copy link
Contributor

@b-dean it's our standard to enable it by default as it fixes a lot of issues in the long term. So it needs to be fixed and we cannot simply just disable it for now as it will likely cause other issues too.

@b-dean
Copy link
Contributor

b-dean commented Apr 19, 2023

Sure, and that's fine, but in the meantime anyone using this cookbook is stuck on Chef 17, which is frustrating.

@ramereth
Copy link
Contributor

@b-dean I completely agree. Anyone is more than welcome to work on a proper fix. I unfortunately don't have any time at the moment.

b-dean added a commit to b-dean/docker that referenced this issue May 20, 2023
not exactly a fix for sous-chefs#1222

Signed-off-by: Ben Dean <ben.dean@finvi.com>
b-dean added a commit to b-dean/docker that referenced this issue May 23, 2023
not exactly a fix for sous-chefs#1222

Signed-off-by: Ben Dean <ben.dean@finvi.com>
b-dean added a commit to b-dean/docker that referenced this issue May 23, 2023
The following resources are now custom resources:
- `docker_installation`
- `docker_installation_package`
- `docker_installation_script`
- `docker_installation_tarball`
- `docker_service`
- `docker_service_base`
- `docker_service_manager`
- `docker_service_manager_execute`
- `docker_service_manager_systemd`

This fixes sous-chefs#1222 so the docker cookbook will work with Chef 18

Signed-off-by: Ben Dean <ben.dean@finvi.com>
b-dean added a commit to b-dean/docker that referenced this issue May 23, 2023
The following resources are now custom resources:
- `docker_installation`
- `docker_installation_package`
- `docker_installation_script`
- `docker_installation_tarball`
- `docker_service`
- `docker_service_base`
- `docker_service_manager`
- `docker_service_manager_execute`
- `docker_service_manager_systemd`

This fixes sous-chefs#1222 so the docker cookbook will work with Chef 18

Signed-off-by: Ben Dean <ben.dean@finvi.com>
@b-dean b-dean mentioned this issue May 23, 2023
3 tasks
b-dean added a commit to b-dean/docker that referenced this issue May 23, 2023
not exactly a fix for sous-chefs#1222

Signed-off-by: Ben Dean <ben.dean@finvi.com>
b-dean added a commit to b-dean/docker that referenced this issue May 23, 2023
The following resources are now custom resources:
- `docker_installation`
- `docker_installation_package`
- `docker_installation_script`
- `docker_installation_tarball`
- `docker_service`
- `docker_service_base`
- `docker_service_manager`
- `docker_service_manager_execute`
- `docker_service_manager_systemd`

This fixes sous-chefs#1222 so the docker cookbook will work with Chef 18

Signed-off-by: Ben Dean <ben.dean@finvi.com>
b-dean added a commit to b-dean/docker that referenced this issue May 23, 2023
not exactly a fix for sous-chefs#1222

Signed-off-by: Ben Dean <ben.dean@finvi.com>
b-dean added a commit to b-dean/docker that referenced this issue May 23, 2023
The following resources are now custom resources:
- `docker_installation`
- `docker_installation_package`
- `docker_installation_script`
- `docker_installation_tarball`
- `docker_service`
- `docker_service_base`
- `docker_service_manager`
- `docker_service_manager_execute`
- `docker_service_manager_systemd`

This fixes sous-chefs#1222 so the docker cookbook will work with Chef 18

Signed-off-by: Ben Dean <ben.dean@finvi.com>
b-dean added a commit to b-dean/docker that referenced this issue May 23, 2023
The following resources are now custom resources:
- `docker_installation`
- `docker_installation_package`
- `docker_installation_script`
- `docker_installation_tarball`
- `docker_service`
- `docker_service_base`
- `docker_service_manager`
- `docker_service_manager_execute`
- `docker_service_manager_systemd`

This fixes sous-chefs#1222 so the docker cookbook will work with Chef 18

Signed-off-by: Ben Dean <ben.dean@finvi.com>
b-dean added a commit to b-dean/docker that referenced this issue May 23, 2023
not exactly a fix for sous-chefs#1222

Signed-off-by: Ben Dean <ben.dean@finvi.com>
b-dean added a commit to b-dean/docker that referenced this issue May 23, 2023
The following resources are now custom resources:
- `docker_installation`
- `docker_installation_package`
- `docker_installation_script`
- `docker_installation_tarball`
- `docker_service`
- `docker_service_base`
- `docker_service_manager`
- `docker_service_manager_execute`
- `docker_service_manager_systemd`

This fixes sous-chefs#1222 so the docker cookbook will work with Chef 18

Signed-off-by: Ben Dean <ben.dean@finvi.com>
b-dean added a commit to b-dean/docker that referenced this issue May 23, 2023
not exactly a fix for sous-chefs#1222

Signed-off-by: Ben Dean <ben.dean@finvi.com>
b-dean added a commit to b-dean/docker that referenced this issue May 23, 2023
The following resources are now custom resources:
- `docker_installation`
- `docker_installation_package`
- `docker_installation_script`
- `docker_installation_tarball`
- `docker_service`
- `docker_service_base`
- `docker_service_manager`
- `docker_service_manager_execute`
- `docker_service_manager_systemd`

This fixes sous-chefs#1222 so the docker cookbook will work with Chef 18

Signed-off-by: Ben Dean <ben.dean@finvi.com>
b-dean added a commit to b-dean/docker that referenced this issue May 23, 2023
The following resources are now custom resources:
- `docker_installation`
- `docker_installation_package`
- `docker_installation_script`
- `docker_installation_tarball`
- `docker_service`
- `docker_service_base`
- `docker_service_manager`
- `docker_service_manager_execute`
- `docker_service_manager_systemd`

This fixes sous-chefs#1222 so the docker cookbook will work with Chef 18

Signed-off-by: Ben Dean <ben.dean@finvi.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants