(PUP-9025) Remove filebucket type server/port defaults

[melissa]

The fileucket type should not have a default set for server or port. We
have logic built in to find both of those pieces of information if they
aren't explicitely set in the resource manifest. We should be using that
default logic rather than arbitrarily setting these defaults for the user.

This wouldn't be in such a compicated state currently if it weren't for
the confusion between server and server_list. Right now, puppet agent
and friends prefer server_list over server. The filebucket application
also prefers server_list over server. If server_list is not set,
we should use server.

Removing these defaults, we get closer parity between how the filebucket
application and the filebucket resource behave when picking what server
to hit.

[melissa]

So, Josh is right. Before this change, if both server_list and server are set, and the server is not specified in the manifest, server is used. After this change, when both server_list and server are set, then the first value of server_list is used.

In this test, here are what server and server_list are set to:

[0] Melissa@bubba:/Users/Melissa/puppet:(ticket/5.5.x/PUP-9025-filebucket-server-list)$ be ./bin/puppet config print server_list
[["server_list_1"], ["server_list_2"]]
[0] Melissa@bubba:/Users/Melissa/puppet:(ticket/5.5.x/PUP-9025-filebucket-server-list)$ be ./bin/puppet config print server
server1

with the change in this pr:

[0] Melissa@bubba:/Users/Melissa/puppet:(ticket/5.5.x/PUP-9025-filebucket-server-list)$ be ./bin/puppet apply -e "filebucket {'main': path => false} F
ile {'/tmp/foo': backup => main, ensure => present, content => 'foikjnllkjijlijkkjnj'}"
Notice: Compiled catalog for bubba.local in environment production in 0.04 seconds
Error: Could not back up /tmp/foo: Failed to open TCP connection to server_list_1:8140 (getaddrinfo: nodename nor servname provided, or not known)
Error: Could not back up /tmp/foo: Failed to open TCP connection to server_list_1:8140 (getaddrinfo: nodename nor servname provided, or not known)
Error: /Stage[main]/Main/File[/tmp/foo]/content: change from '{md5}7d70663568cac5af684503681e3a4d41' to '{md5}591978f107b6b293ea865595a99ca0b1' failed
: Could not back up /tmp/foo: Failed to open TCP connection to server_list_1:8140 (getaddrinfo: nodename nor servname provided, or not known)
Notice: Applied catalog in 0.06 seconds

without this change:

[0] Melissa@bubba:/Users/Melissa/puppet:(ticket/5.5.x/PUP-9025-filebucket-server-list)$ be ./bin/puppet apply -e "filebucket {'main': path => false} F
ile {'/tmp/foo': backup => main, ensure => present, content => 'foikjnllkjijlijkkjnkjnluj'}"
Notice: Compiled catalog for bubba.local in environment production in 0.04 seconds
Error: Could not back up /tmp/foo: Failed to open TCP connection to server1:8140 (getaddrinfo: nodename nor servname provided, or not known)
Error: Could not back up /tmp/foo: Failed to open TCP connection to server1:8140 (getaddrinfo: nodename nor servname provided, or not known)
Error: /Stage[main]/Main/File[/tmp/foo]/content: change from '{md5}7d70663568cac5af684503681e3a4d41' to '{md5}6a751dca34b0ed3007a3fd37d33c48cd' failed
: Could not back up /tmp/foo: Failed to open TCP connection to server1:8140 (getaddrinfo: nodename nor servname provided, or not known)
Notice: Applied catalog in 0.07 seconds

[melissa]

jenkins please test this

[melissa]

This may not be the most ideal thing to merge into a z release, as @joshcooper mentioned. Let's make sure this is what we want to do.

[melissa]

Since finding a default server is logic that happens elsewhere, I wasn't sure if we need/want tests for which server is picked up

[puppetcla]

CLA signed by all contributors.

[melissa]

I wasn't sure if I should add any indication of the logic the indirector goes through to return a server to use

[joshcooper]

jenkins please test this with servertests

[joshcooper]

This is unrelated to this PR, but this output is unexpected to me:

$ be ./bin/puppet config print server_list
[["server_list_1"], ["server_list_2"]]

I'd expect the setting to be printed as it would appear in puppet.conf. I think it's because the server_list_setting needs to implement the print method. And it looks like this is a general problem for settings deriving from :array, eg

$ bx puppet config print supported_checksum_types --supported_checksum_types md5,sha256
["md5", "sha256"]

I'd expect the ArraySetting to implement print to join using comma:

def print(value)
  vals = super(value)
  vals.join(',')
end

And for ServerListSetting to override it so that each host and optional port are first joined with ':', and then that result is comma-joined:

def print(value)
  vals = munge(value)
  super(vals.map { |v| v.join(':') })
end

Could you file a new ticket for that and add it to the epic?

[melissa]

Maybe I defined server_list incorrectly?

[1] Melissa@bubba:/Users/Melissa/puppet:(5.5.x)$ be ./bin/puppet config set server_list server_list_1,server_list_2
[0] Melissa@bubba:/Users/Melissa/puppet:(5.5.x)$ be ./bin/puppet config print server_list
[["server_list_1"], ["server_list_2"]]
[0] Melissa@bubba:/Users/Melissa/puppet:(5.5.x)$ cat ~/.puppetlabs/etc/puppet/puppet.conf
[main]
server = server1
server_list = server_list_1,server_list_2

Otherwise, I'm definitely happy to open a ticket

[melissa]

https://tickets.puppetlabs.com/browse/PUP-9495

[joshcooper]

Verified if an explicit server is omitted from the filebucket resource, then the agent will use the "currently selected" server when backing up files during catalog application. The "currently selected" server is either the first available server from server_list or server, in that order.

filebucket { 'main':
  path   => false,                # This is required for remote filebuckets.
}

File { backup => main, }

file { '/tmp/filebucket':
  ensure => file,
  content => $facts['content']
}

On the other hand, if an explicit server is specified in the filebucket resource, then the agent will use that always:

...

filebucket { 'main':
  path   => false,                # This is required for remote filebuckets.
  server => 'puppet.example.com', # Optional; defaults to the configured puppet master.
}