engine: adding poll function during progressTask #1876
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| } | ||
|
|
||
| for _, dependency := range target.DependsOn { | ||
| dependencyContainer, ok := existingContainers[dependency.Container] | ||
| if !ok { | ||
| return fmt.Errorf("dependency graph: container ordering dependency [%v] for target [%v] does not exist.", dependencyContainer, target) | ||
| return nil,fmt.Errorf("dependency graph: container ordering dependency [%v] for target [%v] does not exist.", dependencyContainer, target) |
There was a problem hiding this comment.
Not sure but do you want to return the &dependency in here?
There was a problem hiding this comment.
I don't think so, since this function is only validating that its possible to resolve.
ubhattacharjya
approved these changes
Feb 21, 2019
aaithal
reviewed
Feb 22, 2019
aaithal
reviewed
Feb 22, 2019
petderek
force-pushed
the
container-ordering-task-sync
branch
from
85397fb to
22391c5
Compare
Prior to this commit, we only tracked state we explicitly tried to
change when the task was starting. We did not respond to the event
stream or any other source of information from Docker. This means
that when we are waiting for certain dependency conditions
("SUCCESS", "COMPLETE", or "HEALTHY") the task progression logic
does not update the agent-internal model of container state.
Since we rely on that state for determining when conditions are
met, tasks would get stuck in infinite startup loops. This change
adds a call to engine.checkTaskState(), which explicity updates
any changed container state. We only call this function if we know
that we are waiting on the aforementioned subset of dependency
conditions.
Co-authored-by: Utsa Bhattacharjya <[email protected]>
petderek
force-pushed
the
container-ordering-task-sync
branch
from
22391c5 to
4c57056
Compare
|
Merging this, will do integ tests on a different PR before we merge this to dev |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This ensures that SUCCESS and HEALTHY conditions are reachable
when progressTask sees no activity.
Implementation details
Prior to this commit, we only tracked state we explicitly tried to
change when the task was starting. We did not respond to the event
stream or any other source of information from Docker. This means
that when we are waiting for certain dependency conditions
("SUCCESS", "COMPLETE", or "HEALTHY") the task progression logic
does not update the agent-internal model of container state.
Since we rely on that state for determining when conditions are
met, tasks would get stuck in infinite startup loops. This change
adds a call to engine.checkTaskState(), which explicity updates
any changed container state. We only call this function if we know
that we are waiting on the aforementioned subset of dependency
conditions.
Testing
We manually tested use cases involving "success" and "healthy".
Integ tests are on the way.
New tests cover the changes: WIP, will be added to this PR or another one depending on how lengthly they get.
Description for the changelog
Licensing
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.