search

sous-chefs / samba

Development repository for the samba cookbook
https://supermarket.chef.io/cookbooks/samba
Apache License 2.0
30 stars 55 forks source link

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

Open jayhendren opened 5 years ago

jayhendren commented 5 years ago

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 commented 5 years ago

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.

jayhendren commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

jayhendren commented 5 years ago

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 commented 5 years ago

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?