samba icon indicating copy to clipboard operation
samba copied to clipboard

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

Open jayhendren opened this issue 6 years ago • 7 comments

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.

jayhendren avatar Mar 19 '19 17:03 jayhendren

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.

damacus avatar Mar 19 '19 19:03 damacus

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.

jayhendren avatar Mar 19 '19 21:03 jayhendren

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.

damacus avatar Mar 20 '19 07:03 damacus

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.

jayhendren avatar Mar 20 '19 08:03 jayhendren

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

damacus avatar Mar 21 '19 22:03 damacus

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:

  • https://www.samba.org/samba/docs/current/man-html/smb.conf.5.html (section "include (S)")
  • https://serverfault.com/a/649619/213070

jayhendren avatar Mar 22 '19 21:03 jayhendren

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 avatar Apr 08 '19 10:04 damacus