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

Fix initial start of etcd only nodes #3748

Merged
merged 3 commits into from
Aug 3, 2021

Conversation

galal-hussein
Copy link
Contributor

@galal-hussein galal-hussein commented Aug 2, 2021

Signed-off-by: galal-hussein [email protected]

Proposed Changes

Fix initial start of systemd notify of etcd only nodes

Types of Changes

bug fix

Linked Issues

@galal-hussein galal-hussein requested a review from a team as a code owner August 2, 2021 23:15
@codecov-commenter
Copy link

codecov-commenter commented Aug 2, 2021

Codecov Report

Merging #3748 (15e5c64) into master (429af17) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3748      +/-   ##
==========================================
- Coverage   11.56%   11.56%   -0.01%     
==========================================
  Files         136      136              
  Lines        8810     8812       +2     
==========================================
  Hits         1019     1019              
- Misses       7570     7572       +2     
  Partials      221      221              
Flag Coverage Δ
inttests 0.62% <0.00%> (-0.01%) ⬇️
unittests 11.28% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/agent/run.go 0.00% <0.00%> (ø)
pkg/cli/server/server.go 0.00% <0.00%> (ø)

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 429af17...15e5c64. Read the comment docs.

@brandond
Copy link
Member

brandond commented Aug 2, 2021

Can you try this instead:

  • Don't revert the change to pkg/cli/server/server.go
  • Remove the the immediate READY notification if cfg.ETCDAgent is set in pkg/agent/run.go.
  • After setupTunnelAndRunAgent returns, if cfg.ETCDAgent is set then reset the variable by calling os.Setenv("NOTIFY_SOCKET", notifySocket).
  • Down at the bottom make the os.Setenv and systemd.SdNotify calls conditional on !cfg.ETCDAgent.

This should have a net effect of resetting the environment variable so that the server goroutine can send the notification when etcd becomes ready (on etcd-only servers), and preventing the agent from sending a second extra notification later when the apiserver comes up.

Copy link
Member

@brandond brandond left a comment

Choose a reason for hiding this comment

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

^^

Signed-off-by: galal-hussein <[email protected]>
Signed-off-by: galal-hussein <[email protected]>
@galal-hussein galal-hussein merged commit 2069cdf into k3s-io:master Aug 3, 2021
dereknola pushed a commit to dereknola/k3s that referenced this pull request Aug 9, 2021
* Fix initial start of etcd only nodes

Signed-off-by: galal-hussein <[email protected]>

* more fixes

Signed-off-by: galal-hussein <[email protected]>

* more fixes

Signed-off-by: galal-hussein <[email protected]>
brandond pushed a commit to brandond/k3s that referenced this pull request Aug 25, 2021
* Fix initial start of etcd only nodes

Signed-off-by: galal-hussein <[email protected]>

* more fixes

Signed-off-by: galal-hussein <[email protected]>

* more fixes

Signed-off-by: galal-hussein <[email protected]>
(cherry picked from commit 2069cdf)
Signed-off-by: Brad Davidson <[email protected]>
@brandond brandond mentioned this pull request Aug 25, 2021
brandond pushed a commit to brandond/k3s that referenced this pull request Apr 15, 2022
* Fix initial start of etcd only nodes

Signed-off-by: galal-hussein <[email protected]>

* more fixes

Signed-off-by: galal-hussein <[email protected]>

* more fixes

Signed-off-by: galal-hussein <[email protected]>
(cherry picked from commit 2069cdf)
brandond added a commit to brandond/k3s that referenced this pull request Apr 15, 2022
Signed-off-by: Brad Davidson <[email protected]>
Signed-off-by: galal-hussein <[email protected]>
(cherry picked from commit 2069cdf)
brandond added a commit that referenced this pull request Apr 15, 2022
Signed-off-by: Brad Davidson <[email protected]>
Signed-off-by: galal-hussein <[email protected]>
(cherry picked from commit 2069cdf)
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