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

(MODULES-11048) task to remove local filebucket #550

Merged
merged 1 commit into from
Apr 20, 2021

Conversation

gimmyxd
Copy link
Contributor

@gimmyxd gimmyxd commented Apr 15, 2021

This adds a task that removes the local filebucket
which was disabled by default in Puppet 7.
The location of filebucket is determined using the
clientbucketdir puppet config.

❯ bolt task show puppet_agent::delete_local_filebucket
puppet_agent::delete_local_filebucket
  Removes the local filebucket

Usage
  bolt task run puppet_agent::delete_local_filebucket --targets <targets>
  [force=<value>]

Parameters
  force  Optional[Boolean]
    ignore nonexistent files and errors

Tested on Windows:

$ ls  /cygdrive/c/ProgramData/PuppetLabs/puppet/cache/clientbucket
ls: cannot access /cygdrive/c/ProgramData/PuppetLabs/puppet/cache/clientbucket: No such file or directory

❯ bolt task run puppet_agent::delete_local_filebucket
 {"_error"=>{"msg"=>"Errno::ENOENT: No such file or directory @ apply2files - C:/ProgramData/PuppetLabs/puppet/cache/clientbucket", "kind"=>"puppet_agent/cannot-remove-error", "details"=>{}}}

❯ bolt task run puppet_agent::delete_local_filebucket force=true
   {:success=>true}

$ mkdir -p /cygdrive/c/ProgramData/PuppetLabs/puppet/cache/clientbucket

❯ bolt task run puppet_agent::delete_local_filebucket
  {:success=>true}
  
$ ls  /cygdrive/c/ProgramData/PuppetLabs/puppet/cache/clientbucket
ls: cannot access /cygdrive/c/ProgramData/PuppetLabs/puppet/cache/clientbucket: No such file or directory

@gimmyxd gimmyxd requested a review from a team as a code owner April 15, 2021 12:23
@gimmyxd gimmyxd requested a review from a team April 15, 2021 12:23
@gimmyxd gimmyxd requested a review from joshcooper April 15, 2021 14:06
override_locale: false,
}

command = "#{puppet_bin} config print clientbucketdir"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@joshcooper thoughts on this? Could this not return the correct path if clientbucketdir is set in a different section?

Copy link
Contributor Author

@gimmyxd gimmyxd Apr 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clientbucketdir can also be found without shelling out to puppet config print.

Puppet.initialize_settings
Puppet[:clientbucketdir]

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like clientbucketdir is defined under the agent section in defaults.rb. However, the puppet filebucket application doesn't enforce a specific run_mode, so it will ignore the value of clientbucketdir if set under [agent].

Using this logic, I think we can rely on the initialize_settings approach as it would be a bit faster.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One downside with initialize_settings is it can only be called once:

irb(main):001:0> require 'puppet'
=> true
irb(main):002:0> Puppet.initialize_settings
=> {}
irb(main):003:0> Puppet.initialize_settings
Traceback (most recent call last):
       16: from /home/josh/.rbenv/versions/2.7.3/lib/ruby/gems/2.7.0/gems/bundler-2.2.16/lib/bundler/vendor/thor/lib/thor/base.rb:485:in `start'
       15: from /home/josh/.rbenv/versions/2.7.3/lib/ruby/gems/2.7.0/gems/bundler-2.2.16/lib/bundler/cli.rb:30:in `dispatch'
       14: from /home/josh/.rbenv/versions/2.7.3/lib/ruby/gems/2.7.0/gems/bundler-2.2.16/lib/bundler/vendor/thor/lib/thor.rb:392:in `dispatch'
       13: from /home/josh/.rbenv/versions/2.7.3/lib/ruby/gems/2.7.0/gems/bundler-2.2.16/lib/bundler/vendor/thor/lib/thor/invocation.rb:127:in `invoke_command'
       12: from /home/josh/.rbenv/versions/2.7.3/lib/ruby/gems/2.7.0/gems/bundler-2.2.16/lib/bundler/vendor/thor/lib/thor/command.rb:27:in `run'
       11: from /home/josh/.rbenv/versions/2.7.3/lib/ruby/gems/2.7.0/gems/bundler-2.2.16/lib/bundler/cli.rb:494:in `exec'
       10: from /home/josh/.rbenv/versions/2.7.3/lib/ruby/gems/2.7.0/gems/bundler-2.2.16/lib/bundler/cli/exec.rb:28:in `run'
        9: from /home/josh/.rbenv/versions/2.7.3/lib/ruby/gems/2.7.0/gems/bundler-2.2.16/lib/bundler/cli/exec.rb:63:in `kernel_load'
        8: from /home/josh/.rbenv/versions/2.7.3/lib/ruby/gems/2.7.0/gems/bundler-2.2.16/lib/bundler/cli/exec.rb:63:in `load'
        7: from /home/josh/.rbenv/versions/2.7.3/bin/irb:23:in `<top (required)>'
        6: from /home/josh/.rbenv/versions/2.7.3/bin/irb:23:in `load'
        5: from /home/josh/.rbenv/versions/2.7.3/lib/ruby/gems/2.7.0/gems/irb-1.2.6/exe/irb:11:in `<top (required)>'
        4: from (irb):3
        3: from /home/josh/work/puppet/lib/puppet.rb:153:in `initialize_settings'
        2: from /home/josh/work/puppet/lib/puppet.rb:178:in `do_initialize_settings_for_run_mode'
        1: from /home/josh/work/puppet/lib/puppet/settings.rb:305:in `initialize_global_settings'
Puppet::DevError (Attempting to initialize global default settings more than once!)

So it seems like there could be issues if this task was called twice in a plan (seems unlikely) or some other task also called initialize_settings?

I'd be inclined to just shell out.

@gimmyxd gimmyxd force-pushed the MODULES-11048 branch 2 times, most recently from 8292f29 to 191ae7f Compare April 20, 2021 05:41
) unless puppet_bin_present?

begin
FileUtils.remove_dir(clientbucketdir, force)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may not worth fixing it, but if the clientbucketdir is removed, the puppet command listing local filebuckets may error:

scrawny-tumbler:~ # find /opt/puppetlabs/puppet/cache/clientbucket/                 
find: `/opt/puppetlabs/puppet/cache/clientbucket/': No such file or directory
scrawny-tumbler:~ # puppet filebucket -l list
Error: Could not run: File not found
scrawny-tumbler:~ # mkdir /opt/puppetlabs/puppet/cache/clientbucket/
scrawny-tumbler:~ # puppet filebucket -l list
scrawny-tumbler:~ # puppet filebucket backup /etc/passwd --local
scrawny-tumbler:~ #

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems that puppet will create the directory again on the next run:

Debug: /File[/opt/puppetlabs/puppet/cache/clientbucket]: Adding autorequire relationship with File[/opt/puppetlabs/puppet/cache]
...
Debug: /File[/opt/puppetlabs/puppet/cache/clientbucket]/ensure: created

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should just remove the directory contents then?

Copy link
Contributor

@ciprianbadescu ciprianbadescu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good to merge, one optional comment

@ciprianbadescu ciprianbadescu requested a review from a team April 20, 2021 11:26
This adds a task that removes the local filebucket
which was disabled by default in Puppet 7.
The location of filebucket is determine using the
`clientbucketdir` puppet config.

```
❯ bolt task show puppet_agent::delete_local_filebucket
puppet_agent::delete_local_filebucket
  Removes the local filebucket

Usage
  bolt task run puppet_agent::delete_local_filebucket --targets <targets>
  [force=<value>]

Parameters
  force  Optional[Boolean]
    ignore nonexistent files and errors
```
@gimmyxd gimmyxd closed this Apr 20, 2021
@gimmyxd gimmyxd reopened this Apr 20, 2021
@ciprianbadescu ciprianbadescu merged commit 3669c16 into puppetlabs:main Apr 20, 2021
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

Successfully merging this pull request may close these issues.

4 participants