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

Add docker swarm join-tokens as facts #651

Merged

Conversation

oschusler
Copy link
Contributor

Fix for issue #638.

This PR adds 2 facts to PuppetDB (worker_join_token and manager_join_token). The puppet agent will have to be run twice, once to setup the docker manager, and a second time to send the updated fact to PuppetDB. Combined with the following snippet, a docker cluster can be started up, unsupervised:

node 'manager' {
  docker::swarm {'cluster_manager':
    init           => true,
    advertise_addr => "${::ipaddress}",
    listen_addr    => "${::ipaddress}",
    require        => Class['docker'],
  }

  @@docker::swarm {'cluster_worker':
    join           => true,
    manager_ip     => "${ipaddress}",
    token          => "${worker_join_token}",
    tag            => "cluster_join_command",
    require        => Class['docker'],
  }
}

node 'worker' {
  Docker::Swarm<<| tag == 'cluster_join_command' |>> {
    advertise_addr => "${ipaddress}",
    listen_addr    => "${ipaddress}",
  }
}

@oschusler oschusler requested a review from a team as a code owner August 17, 2020 08:01
@oschusler
Copy link
Contributor Author

Travis seems to time out only on PUPPET_GEM_VERSION="~> 6.0" CHECK=parallel_spec. It claims not to have received any output in the last 10 minutes, while the output has finished ("Finished in 6 minutes 19 seconds (files took 5 minutes 4 seconds to load)")

@david22swan
Copy link
Member

@oschusler Took a quick look at your pr and it all looks fine but would need some changes before it can be merged in. For a PR such as this both updated test's and docs would be needed, to ensure that the functionality works as is intended and make it so that people can easily understand it's function.
If you can get both of those done I don't see a reason why this should not be merged.

@oschusler
Copy link
Contributor Author

@david22swan thank you for your comments. I will add the documentation and tests. Could you however perhaps explain to me why Travis is failing? I complains that it didn't receive any logging in the last 10 minutes, while the last log statement was that all tests passed.

@codecov-commenter
Copy link

codecov-commenter commented Aug 20, 2020

Codecov Report

Merging #651 into main will decrease coverage by 3.27%.
The diff coverage is 70.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #651      +/-   ##
==========================================
- Coverage   25.64%   22.37%   -3.28%     
==========================================
  Files          20       20              
  Lines         741      751      +10     
==========================================
- Hits          190      168      -22     
- Misses        551      583      +32     
Impacted Files Coverage Δ
lib/facter/docker.rb 42.66% <70.00%> (-40.42%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c3a9f56...8cac029. Read the comment docs.

@david22swan
Copy link
Member

@oschusler From what I know there was a problem with our code coverage tool that was causing it to hang indefinitely. A fix has gone in though so there should be no more problems.

@oschusler oschusler force-pushed the feature/add-join-token-facts branch from 45cea90 to 8cac029 Compare August 24, 2020 13:11
@oschusler
Copy link
Contributor Author

@david22swan I think the PR is ready, could you have a look at it?

@david22swan
Copy link
Member

@oschusler Apologies for the wait, taking a look through I would say I am happy with what you have done and feel good about merging it.
Thanks for the work :)

@david22swan david22swan merged commit d908f5d into puppetlabs:main Sep 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants