-
Notifications
You must be signed in to change notification settings - Fork 25
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
1356721 - Filter/Validate hosts aren't already deploying #1083
Conversation
cfchase
commented
Jul 22, 2016
- Added validation that no other fusor deployment is running.
- Added validation that deployment's hosts are not already managed.
- Added filter for engine/hypervisor discovered hosts to omit hosts associated with a currently running deployment.
deployment.errors[:foreman_task_uuid] << _("Deployment #{other.id}: #{other.name} is already running") | ||
end | ||
end | ||
|
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.
ruby looks good. My biggest worry is we have to wait for one deployment to completely finish before being able to kick the other one off. The biggest issue I see with multiples running at the same time is manage_content failing. But if we truly only support one at a time, this is it.
PR title has a typo: 'FIlter' instead of 'Filter' |
fixed PR title |
@@ -119,7 +124,7 @@ export default Ember.Controller.extend(NeedsDeploymentMixin, { | |||
!trackedHostIds | |||
.filter((hostId) => this.get('hypervisorModelIds').contains(hostId)) | |||
.map((k) => vState.get(k)) | |||
.reduce((lhs, rhs) => lhs && rhs); | |||
.reduce((previousAreTrue, currentValue) => previousAreTrue && currentValue, true); |
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.
Better var names 👍 I was guilty of the lhs/rhs
Added validation that deployment's hosts are not already managed.
hosts associated with a currently running deployment.
I know you were concerned about the promises here, but I actually think it's pretty clean given what had to be done to get the hosts. Read okay to me. ACK ember |