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.

Sign up for GitHub

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

Jump to bottom

FIX: #412 set Restart flag to true before creation (#413, thanks @fabricev) #413

Merged
merged 6 commits into from
Nov 26, 2020
Merged

FIX: #412 set Restart flag to true before creation (#413, thanks @fabricev) #413

merged 6 commits into from
Nov 26, 2020

Conversation

fabricev
Copy link
Contributor

No description provided.

iwilltry42
iwilltry42 reviewed Nov 24, 2020
View reviewed changes
Copy link
Member

@iwilltry42 iwilltry42 left a comment

Choose a reason for hiding this comment

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

LGTM!

@iwilltry42
Copy link
Member

I think we'll need to add this to the single node creation (k3d node create) as well.

@iwilltry42 iwilltry42 added this to the v3.3.1 milestone Nov 24, 2020
@iwilltry42 iwilltry42 added the enhancement New feature or request label Nov 24, 2020
@fabricev
FIX: Add default restart policy for nodes
6eceb6d
@fabricev
ADD: GetNodeStatus exposed in the runtime interface
abfa745 
ADD: Early exit in WaitForLogMessage if restarting
FIX: Sleep(500) inside the loop
@fabricev
Copy link
Contributor Author

fabricev commented Nov 24, 2020 •
edited
Loading

Hello @iwilltry42, as suggested I've implemented the early exit by checking the restarting status in the waitForLogMessage.
I wanted to minimize the changes and I've reused the GetNodeStatus() from Docker runtime, now exposed in the runtime interface
Side note, I'd like to make sure that moving the Sleep() inside the loop is what was initially intended with the Sleep()

iwilltry42
iwilltry42 requested changes Nov 25, 2020
View reviewed changes
Copy link
Member

@iwilltry42 iwilltry42 left a comment

Choose a reason for hiding this comment

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

Some minor improvements requested, but looks good to me :)

pkg/cluster/node.go Outdated Show resolved Hide resolved
pkg/cluster/node.go Show resolved Hide resolved
pkg/types/types.go Outdated Show resolved Hide resolved
@fabricev fabricev requested a review from iwilltry42 November 25, 2020 21:27
iwilltry42
iwilltry42 approved these changes Nov 26, 2020
View reviewed changes
Copy link
Member

@iwilltry42 iwilltry42 left a comment

Choose a reason for hiding this comment

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

LGTM!

@iwilltry42
Copy link
Member

Thanks for your contribution! You may want to use the all-contributors bot to add yourself to the list of contributors shown in the readme for the code you provided 👍

@iwilltry42 iwilltry42 changed the title FIX: #412 set Restart flag to true before creation FIX: #412 set Restart flag to true before creation (#413, thanks @fabricev) Nov 26, 2020
@iwilltry42 iwilltry42 merged commit 2d8d60b into k3d-io:main Nov 26, 2020
@iwilltry42 iwilltry42 self-assigned this Nov 26, 2020
rancherio-gh-m pushed a commit that referenced this pull request Nov 26, 2020
@fabricev
commit 2d8d60b
040e08c 
Author: Fabrice Vergnenegre <[email protected]>
Date:   Thu Nov 26 08:54:06 2020 +0100

    FIX: #412 set Restart flag to true before creation (#413, thanks @fabricev)
@iwilltry42 iwilltry42 modified the milestones: v3.3.1, v3.4.0 Dec 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iwilltry42 iwilltry42 iwilltry42 approved these changes

Assignees

@iwilltry42 iwilltry42

Labels
enhancement New feature or request
Projects
None yet
Milestone
v3.4.0
Development

Successfully merging this pull request may close these issues.
2 participants
@fabricev @iwilltry42