-
Notifications
You must be signed in to change notification settings - Fork 15
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
Adds block to prevent deploying to un-handshake'd VM, shuts node down if first handshake fails #136
Conversation
Signed-off-by: Kevin Hoffman <[email protected]>
@@ -352,133 +355,6 @@ func (m *MachineManager) awaitHandshake(vmid string) { | |||
} | |||
} | |||
|
|||
// Called when the node server gets a log entry via internal NATS. Used to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I kinda liked having all of the MachineManager
code in the same place. Maybe it doesn't matter too much. @jordan-rash do you have an opinion?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I start to itch when we get over the 500 line mark. While yes it's all under the machine manager umbrella, I like having logically related stuff in nearby locations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I start to itch when we get over the 500 line mark. While yes it's all under the machine manager umbrella, I like having logically related stuff in nearby locations.
Makes sense. It would be better if we created a new interface and struct to make things more readable imho instead of separating it into separate files. I will not die on this hill though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can revisit creating an interface for select MachineManager
methods we are moving into machine_mgr_events.go
in a separate PR.
Closes #134
Don't worry about the large "delete" portion of the diff, I just moved a bunch of related functions into their own file to try and keep the
machine_mgr.go
file to a manageable size.