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

Samba shares nuked when unrelated resource fails during chef-client run #96

Open
jayhendren opened this issue Mar 19, 2019 · 7 comments
Labels
Bug Something isn't working Good First Issue An issue which is a good way to get started in the project Waiting on Contributor Awaiting on the person who raised this to update

Comments

@jayhendren
Copy link

Cookbook version

1.2.0

Chef-client version

14.2.0

Platform Details

RHEL 7.6

Scenario:

Steps to Reproduce:

Run the following recipe:

# a wrapper cookbook that installs/configures sssd, firewall, selinux, etc.
include_recipe 'cub_samba::server'

bash 'randomly fail' do
  code 'exit $(($RANDOM % 2))'
end

samba_share 'homes' do
  comment 'Home Directories'
  guest_ok 'no'
  read_only 'no'
  browseable 'no'
  create_directory false
end

Half the time this recipe will execute successfully, causing the correct config file to get written out:

[vagrant@12996bc1-c6ab-4 ~]$ cat /etc/samba/smb.conf
# This is the main Samba configuration file.
#
# It was dynamically generated by Chef on 12996bc1-c6ab-4.AD.COLORADO.EDU
#
# Local modifications will be overwritten by Chef.
#

[global]
  realm = AD.COLORADO.EDU
  password server = *
  encrypt passwords = yes
  kerberos method = system keytab
  workgroup = AD
  server string = %h samba
  security = ADS
  map to guest = Bad User
  interfaces = 10.0.2.15
  hosts allow = 127.0.0.0/8
  load printers = no
  passdb backend = tdbsam
  dns proxy = no
  max log size = 5000
  bind interfaces only = no

    restrict anonymous = 2
    idmap config * : backend = tdb
    idmap config * : range = 20000001-20001000
    idmap config AD:backend = ad
    idmap config AD:schema_mode = rfc2307
    idmap config AD:range = 1000-20000000
    include = /etc/samba/smb-include.conf

#============================ Share Definitions ==============================
  [homes]
    comment = Home Directories
    path = 
    guest ok = no
    printable = no
    write list = 
    create mask = 0744
    directory mask = 0755
    read only = no
    valid users = 
    force group = 
    browseable = no
[vagrant@12996bc1-c6ab-4 ~]$

Half the time this recipe will fail. This causes an incorrect configuration file, missing the shares, to get written out:

[vagrant@12996bc1-c6ab-4 ~]$ cat /etc/samba/smb.conf
# This is the main Samba configuration file.
#
# It was dynamically generated by Chef on 12996bc1-c6ab-4.AD.COLORADO.EDU
#
# Local modifications will be overwritten by Chef.
#

[global]
  realm = AD.COLORADO.EDU
  password server = *
  encrypt passwords = yes
  kerberos method = system keytab
  workgroup = AD
  server string = %h samba
  security = ADS
  map to guest = Bad User
  interfaces = 10.0.2.15
  hosts allow = 127.0.0.0/8
  load printers = no
  passdb backend = tdbsam
  dns proxy = no
  max log size = 5000
  bind interfaces only = no

    restrict anonymous = 2
    idmap config * : backend = tdb
    idmap config * : range = 20000001-20001000
    idmap config AD:backend = ad
    idmap config AD:schema_mode = rfc2307
    idmap config AD:range = 1000-20000000
    include = /etc/samba/smb-include.conf

#============================ Share Definitions ==============================
[vagrant@12996bc1-c6ab-4 ~]$

This recently caused a service interruption on one of our customer's systems. A yum repository resource, unrelated to samba, failed. This caused samba to write out the config without any shares. As a result, all of the customer's shares went away.

Expected Result:

When the recipe fails, existing shares should be remain intact in order to avoid service interruptions.

Actual Result:

When the recipe fails, existing shares are removed from the configuration file, causing service interruptions.

@damacus
Copy link
Member

damacus commented Mar 19, 2019

I suspect this is to do with the accumulator pattern.it sounds like something earlier on in the run is altering the share resource. Then the last share isn't getting a chance to be added to the collection.
This is strictly the correct behaviour. Though that doesn't mean desired!

I would suggest putting the samba server and the share resource together so they fail together, and don't alter the file.

If that doesn't work, or I'm completely off the mark here. Let me know and we cna further dig into this problem.

@xorima xorima added the bug label Mar 19, 2019
@jayhendren
Copy link
Author

Yes, this seems to be a common problem in community cookbooks that use the accumulator pattern.

I believe your suggestion...

...putting the samba server and the share resource together so they fail together, and don't alter the file.

... will work in my particular case. At least, it fixed the issue in the quick-and-dirty testing that I've been doing. However, it doesn't work in all cases. When the accumulated resources are spread out across multiple cookbooks, it becomes impossible to group them together - I ran into this issue with the firewall cookbook, for instance.

@damacus
Copy link
Member

damacus commented Mar 20, 2019

Yeah we really don't like using the accumulator pattern, but are usually forced into it when there is no conf.d directory for the software.

Technically, I'm not sure there's a lot we can do apart from the above suggestion of keeping together resources. Especially if in between those resources you have things that are likely to fail.

@jayhendren
Copy link
Author

smb.conf supports an include directive, so maybe creating a samba.conf.d directory would work even if it's not the default or typical configuration file layout for samba? Just kind of spitballing here - switching to that pattern would obviously be an enormous design change from the current cookbook.

@damacus
Copy link
Member

damacus commented Mar 21, 2019

If we think we can get that working, and you and point me at that documentation! That'd be awesome.

@jayhendren
Copy link
Author

Well it may be a moot point. Turns out smb.conf does not support wildcard includes, only a single file at a time. So even with includes, it would probably be necessary to use an accumulator to build the list of files to include.

References:

@damacus
Copy link
Member

damacus commented Apr 8, 2019

Do we think this is possible to do, or it should be documented it isn't recommended to structure the resources in the way you're doing?

@damacus damacus removed the bug label Apr 8, 2019
@xorima xorima added Bug Something isn't working Good First Issue An issue which is a good way to get started in the project labels May 11, 2019
@damacus damacus added the Waiting on Contributor Awaiting on the person who raised this to update label Aug 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Good First Issue An issue which is a good way to get started in the project Waiting on Contributor Awaiting on the person who raised this to update
Projects
None yet
Development

No branches or pull requests

3 participants