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

Package pinning and auto restart of etcd #420

Merged
merged 20 commits into from
Aug 17, 2020
Merged

Package pinning and auto restart of etcd #420

merged 20 commits into from
Aug 17, 2020

Conversation

scoopex
Copy link
Contributor

@scoopex scoopex commented Jun 13, 2020

This pull request adresses: #417 and #190.

Sorry, i was not successful in executing the litmus tests described in the "Development" section on my ubuntu 18.04 system. "litmus:install_module" dropped very cryptic messages which were not understandable for a litmus newbie.... (see #419)
So probably you can fix potential acceptance test problems to get these useful changes to upstream?

Marc Schoechlin added 3 commits June 13, 2020 08:52
- add option for package pinning on debian and ubuntu to prevent
  accidential upgrades
- make dashboard installation more reliable
- simple reformatting to satisfy formatting rules
- adapt an enhance tests for the changes above
- add /usr/bin to default_path to solve #415
- prevent manual intervention on updates
- improve security using checksumming
- notify services on changes
@scoopex scoopex requested a review from a team as a code owner June 13, 2020 13:27
@codecov-commenter
Copy link

codecov-commenter commented Jun 13, 2020

Codecov Report

Merging #420 into main will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##            main    #420   +/-   ##
=====================================
  Coverage   0.00%   0.00%           
=====================================
  Files          2       2           
  Lines         44      44           
=====================================
  Misses        44      44           

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 9fa9ee1...39b919e. Read the comment docs.

@scoopex scoopex changed the title WIP: Component installation Component installation Jun 20, 2020
@scoopex
Copy link
Contributor Author

scoopex commented Jul 20, 2020

Feedback or merge would be nice :-)

Marc Schoechlin added 3 commits August 4, 2020 08:30
- add option for package pinning on debian and ubuntu to prevent
  accidential upgrades
- make dashboard installation more reliable
- simple reformatting to satisfy formatting rules
- adapt an enhance tests for the changes above
- add /usr/bin to default_path to solve #415
- prevent manual intervention on updates
- improve security using checksumming
- notify services on changes
@scoopex
Copy link
Contributor Author

scoopex commented Aug 4, 2020

Feedback or merge would be nice :-)

Which is worse for this puppet module? No feedback for pullrequests and mergerequests to leave open forever or to apply corrections that have not been reviewed extensively? The current situation is not really satisfying for contributors.

Marc Schoechlin and others added 6 commits August 4, 2020 08:36
- add option for package pinning on debian and ubuntu to prevent
  accidential upgrades
- make dashboard installation more reliable
- simple reformatting to satisfy formatting rules
- adapt an enhance tests for the changes above
- add /usr/bin to default_path to solve #415
- add option for package pinning on debian and ubuntu to prevent
  accidential upgrades
- make dashboard installation more reliable
- simple reformatting to satisfy formatting rules
- adapt an enhance tests for the changes above
- add /usr/bin to default_path to solve #415
Improve initial installation
@david22swan david22swan changed the base branch from master to main August 6, 2020 11:42
@david22swan
Copy link
Member

@scoopex
Making a quick change to point this PR towards the newly defined default main branch as part of our work to remove inappropriate terminology. Apologies if this is inconvenient in any way.

@scoopex
Copy link
Contributor Author

scoopex commented Aug 6, 2020

Great, thanks.

@daianamezdrea
Copy link
Contributor

Hi @scoopex, could you please rebase this PR? Thank you!

Marc Schoechlin added 2 commits August 10, 2020 12:57
- prevent manual intervention on updates
- improve security using checksumming
- notify services on changes
@scoopex
Copy link
Contributor Author

scoopex commented Aug 10, 2020

Rebased :-)

@sanfrancrisko
Copy link
Contributor

Hi @scoopex - thanks for your patience; the team has been down on resources but up on work, unfortunately. We're trying to get back on top of things.

We're reviewing the changes on the PR at the moment, however, there seems to be some merge conflicts - would you be able to take a look at this?

Marc Schoechlin added 2 commits August 10, 2020 16:14
- prevent manual intervention on updates
- improve security using checksumming
- notify services on changes
@scoopex
Copy link
Contributor Author

scoopex commented Aug 10, 2020

Sorry, i forgot that upstream/master is renamed to upstream/main :-)

@sanfrancrisko
Copy link
Contributor

Hi @scoopex - we're hoping to get your PR over the line today. Again, thanks for your patience.

There were a few PRs merged in recently that are now causing conflicts on the branch. Could I ask you to kindly rebase with puppetlabs/puppetlabs-kubernetes:main again and we'll look to get our remaining testing done and the PR merged later today?

Many thanks!

Marc Schoechlin added 3 commits August 13, 2020 13:32
- prevent manual intervention on updates
- improve security using checksumming
- notify services on changes
- add option for package pinning on debian and ubuntu to prevent
  accidential upgrades
- make dashboard installation more reliable
- simple reformatting to satisfy formatting rules
- adapt an enhance tests for the changes above
- add /usr/bin to default_path to solve #415
@scoopex
Copy link
Contributor Author

scoopex commented Aug 13, 2020

Rebased the branch. I tested the merged state with my own kitchen test setup. It seems that the conflicts were originated in the apply order of my various pull requests in combination with the puppetlabs code formatting rules which enforce large changesets almost every time a developer touches the parameters of a class.

I am now on holiday until 1.9.2020, so I will probably only be able to give feedback on questions after this date.

@sanfrancrisko
Copy link
Contributor

sanfrancrisko commented Aug 13, 2020

Thanks @scoopex - we're hoping to get our own testing wrapped up today and the PR merged later. There's been a few environmental issues frustrating that process, at the moment.

Yeah, I noticed there was a lot of churn around the parameters. I was trying to resolve the conflicts myself, but, given the size of the PR, I thought it would be safer if you were to resolve them. Normally, if we have the cycles, we're happy to do that sort of stuff ourselves and not call on the original contributor.

If any change to the parameters is resulting in these sorts of changes then I'll raise a ticket to see if we can fix up the branch to a state that doesn't mean a load of churn on each PR.

P.S. Enjoy your holiday! 😄

@sanfrancrisko sanfrancrisko changed the title Component installation Package pinning and auto restart of etcd Aug 17, 2020
@sanfrancrisko sanfrancrisko merged commit 16101c9 into puppetlabs:main Aug 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants