Skip to content

Commit

Permalink
Merge pull request #1876 from petderek/container-ordering-task-sync
Browse files Browse the repository at this point in the history 
engine: adding poll function during progressTask
  • Loading branch information
@petderek
petderek authored Feb 22, 2019
2 parents 6dcd3e3 + 4c57056 commit 3bc095b
Show file tree
Hide file tree
Showing 6 changed files with 120 additions and 89 deletions.
26 changes: 13 additions & 13 deletions agent/engine/dependencygraph/graph.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ func dependenciesCanBeResolved(target *apicontainer.Container, by []*apicontaine
nameMap[cont.Name] = cont
}

if err := verifyContainerOrderingStatusResolvable(target, nameMap, containerOrderingDependenciesCanResolve); err != nil {
if _, err := verifyContainerOrderingStatusResolvable(target, nameMap, containerOrderingDependenciesCanResolve); err != nil {
return false
}
return verifyStatusResolvable(target, nameMap, target.SteadyStateDependencies, onSteadyStateCanResolve)
Expand All @@ -122,9 +122,9 @@ func DependenciesAreResolved(target *apicontainer.Container,
by []*apicontainer.Container,
id string,
manager credentials.Manager,
resources []taskresource.TaskResource) error {
resources []taskresource.TaskResource) (*apicontainer.DependsOn, error) {
if !executionCredentialsResolved(target, id, manager) {
return CredentialsNotResolvedErr
return nil, CredentialsNotResolvedErr
}

nameMap := make(map[string]*apicontainer.Container)
Expand All @@ -141,18 +141,18 @@ func DependenciesAreResolved(target *apicontainer.Container,
resourcesMap[resource.GetName()] = resource
}

if err := verifyContainerOrderingStatusResolvable(target, nameMap, containerOrderingDependenciesIsResolved); err != nil {
return err
if blocked, err := verifyContainerOrderingStatusResolvable(target, nameMap, containerOrderingDependenciesIsResolved); err != nil {
return blocked, err
}

if !verifyStatusResolvable(target, nameMap, target.SteadyStateDependencies, onSteadyStateIsResolved) {
return DependentContainerNotResolvedErr
return nil, DependentContainerNotResolvedErr
}
if err := verifyTransitionDependenciesResolved(target, nameMap, resourcesMap); err != nil {
return err
return nil, err
}

return nil
return nil, nil
}

func linksToContainerNames(links []string) []string {
Expand Down Expand Up @@ -205,25 +205,25 @@ func verifyStatusResolvable(target *apicontainer.Container, existingContainers m
// (map from name to container). The `resolves` function passed should return true if the named container is resolved.

func verifyContainerOrderingStatusResolvable(target *apicontainer.Container, existingContainers map[string]*apicontainer.Container,
resolves func(*apicontainer.Container, *apicontainer.Container, string) bool) error {
resolves func(*apicontainer.Container, *apicontainer.Container, string) bool) (*apicontainer.DependsOn, error) {

targetGoal := target.GetDesiredStatus()
if targetGoal != target.GetSteadyStateStatus() && targetGoal != apicontainerstatus.ContainerCreated {
// A container can always stop, die, or reach whatever other state it
// wants regardless of what dependencies it has
return nil
return nil, nil
}

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)
}
if !resolves(target, dependencyContainer, dependency.Condition) {
return fmt.Errorf("dependency graph: failed to resolve the container ordering dependency [%v] for target [%v]", dependencyContainer, target)
return &dependency, fmt.Errorf("dependency graph: failed to resolve the container ordering dependency [%v] for target [%v]", dependencyContainer, target)
}
}
return nil
return nil, nil
}

func verifyTransitionDependenciesResolved(target *apicontainer.Container,
Expand Down
36 changes: 20 additions & 16 deletions agent/engine/dependencygraph/graph_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ func TestDependenciesAreResolvedWhenSteadyStateIsRunning(t *testing.T) {
},
},
}
err := DependenciesAreResolved(task.Containers[0], task.Containers, "", nil, nil)
_, err := DependenciesAreResolved(task.Containers[0], task.Containers, "", nil, nil)
assert.NoError(t, err, "One container should resolve trivially")

// Webserver stack
Expand All @@ -138,28 +138,28 @@ func TestDependenciesAreResolvedWhenSteadyStateIsRunning(t *testing.T) {
},
}

err = DependenciesAreResolved(php, task.Containers, "", nil, nil)
_, err = DependenciesAreResolved(php, task.Containers, "", nil, nil)
assert.Error(t, err, "Shouldn't be resolved; db isn't running")

err = DependenciesAreResolved(db, task.Containers, "", nil, nil)
_, err = DependenciesAreResolved(db, task.Containers, "", nil, nil)
assert.Error(t, err, "Shouldn't be resolved; dbdatavolume isn't created")

err = DependenciesAreResolved(dbdata, task.Containers, "", nil, nil)
_, err = DependenciesAreResolved(dbdata, task.Containers, "", nil, nil)
assert.NoError(t, err, "data volume with no deps should resolve")

dbdata.SetKnownStatus(apicontainerstatus.ContainerCreated)
err = DependenciesAreResolved(php, task.Containers, "", nil, nil)
_, err = DependenciesAreResolved(php, task.Containers, "", nil, nil)
assert.Error(t, err, "Php shouldn't run, db is not created")

db.SetKnownStatus(apicontainerstatus.ContainerCreated)
err = DependenciesAreResolved(php, task.Containers, "", nil, nil)
_, err = DependenciesAreResolved(php, task.Containers, "", nil, nil)
assert.Error(t, err, "Php shouldn't run, db is not running")

err = DependenciesAreResolved(db, task.Containers, "", nil, nil)
_, err = DependenciesAreResolved(db, task.Containers, "", nil, nil)
assert.NoError(t, err, "db should be resolved, dbdata volume is Created")
db.SetKnownStatus(apicontainerstatus.ContainerRunning)

err = DependenciesAreResolved(php, task.Containers, "", nil, nil)
_, err = DependenciesAreResolved(php, task.Containers, "", nil, nil)
assert.NoError(t, err, "Php should resolve")
}

Expand All @@ -175,16 +175,20 @@ func TestRunDependencies(t *testing.T) {
SteadyStateDependencies: []string{"a"},
}
task := &apitask.Task{Containers: []*apicontainer.Container{c1, c2}}
_, err := DependenciesAreResolved(c2, task.Containers, "", nil, nil)
assert.Error(t, err, "Dependencies should not be resolved")

assert.Error(t, DependenciesAreResolved(c2, task.Containers, "", nil, nil), "Dependencies should not be resolved")
task.Containers[1].SetDesiredStatus(apicontainerstatus.ContainerRunning)
assert.Error(t, DependenciesAreResolved(c2, task.Containers, "", nil, nil), "Dependencies should not be resolved")
_, err = DependenciesAreResolved(c2, task.Containers, "", nil, nil)
assert.Error(t, err, "Dependencies should not be resolved")

task.Containers[0].SetKnownStatus(apicontainerstatus.ContainerRunning)
assert.NoError(t, DependenciesAreResolved(c2, task.Containers, "", nil, nil), "Dependencies should be resolved")
_, err = DependenciesAreResolved(c2, task.Containers, "", nil, nil)
assert.NoError(t, err, "Dependencies should be resolved")

task.Containers[1].SetDesiredStatus(apicontainerstatus.ContainerCreated)
assert.NoError(t, DependenciesAreResolved(c1, task.Containers, "", nil, nil), "Dependencies should be resolved")
_, err = DependenciesAreResolved(c1, task.Containers, "", nil, nil)
assert.NoError(t, err, "Dependencies should be resolved")
}

func TestRunDependenciesWhenSteadyStateIsResourcesProvisionedForOneContainer(t *testing.T) {
Expand All @@ -210,11 +214,11 @@ func TestRunDependenciesWhenSteadyStateIsResourcesProvisionedForOneContainer(t *
continue
}
container.SteadyStateDependencies = []string{"pause"}
err := DependenciesAreResolved(container, task.Containers, "", nil, nil)
_, err := DependenciesAreResolved(container, task.Containers, "", nil, nil)
assert.Error(t, err, "Shouldn't be resolved; pause isn't running")
}

err := DependenciesAreResolved(pause, task.Containers, "", nil, nil)
_, err := DependenciesAreResolved(pause, task.Containers, "", nil, nil)
assert.NoError(t, err, "Pause container's dependencies should be resolved")

// Transition pause container to RUNNING
Expand All @@ -228,13 +232,13 @@ func TestRunDependenciesWhenSteadyStateIsResourcesProvisionedForOneContainer(t *
}
// Assert that dependencies remain unresolved until the pause container reaches
// RESOURCES_PROVISIONED
err = DependenciesAreResolved(container, task.Containers, "", nil, nil)
_, err = DependenciesAreResolved(container, task.Containers, "", nil, nil)
assert.Error(t, err, "Shouldn't be resolved; pause isn't running")
}
pause.KnownStatusUnsafe = apicontainerstatus.ContainerResourcesProvisioned
// Dependecies should be resolved now that the 'pause' container has
// transitioned into RESOURCES_PROVISIONED
err = DependenciesAreResolved(php, task.Containers, "", nil, nil)
_, err = DependenciesAreResolved(php, task.Containers, "", nil, nil)
assert.NoError(t, err, "Php should resolve")
}

Expand Down
64 changes: 32 additions & 32 deletions agent/engine/docker_image_manager_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,8 @@ func TestRecordContainerReferenceInspectError(t *testing.T) {
client := mock_dockerapi.NewMockDockerClient(ctrl)

imageManager := &dockerImageManager{
client: client,
state: dockerstate.NewTaskEngineState(),
client: client,
state: dockerstate.NewTaskEngineState(),
minimumAgeBeforeDeletion: config.DefaultImageDeletionAge,
numImagesToDelete: config.DefaultNumImagesToDeletePerCycle,
imageCleanupTimeInterval: config.DefaultImageCleanupTimeInterval,
Expand Down Expand Up @@ -179,8 +179,8 @@ func TestRecordContainerReferenceWithNoImageName(t *testing.T) {
client := mock_dockerapi.NewMockDockerClient(ctrl)

imageManager := &dockerImageManager{
client: client,
state: dockerstate.NewTaskEngineState(),
client: client,
state: dockerstate.NewTaskEngineState(),
minimumAgeBeforeDeletion: config.DefaultImageDeletionAge,
numImagesToDelete: config.DefaultNumImagesToDeletePerCycle,
imageCleanupTimeInterval: config.DefaultImageCleanupTimeInterval,
Expand Down Expand Up @@ -406,8 +406,8 @@ func TestRemoveContainerReferenceFromImageStateWithNoReference(t *testing.T) {
client := mock_dockerapi.NewMockDockerClient(ctrl)

imageManager := &dockerImageManager{
client: client,
state: dockerstate.NewTaskEngineState(),
client: client,
state: dockerstate.NewTaskEngineState(),
minimumAgeBeforeDeletion: config.DefaultImageDeletionAge,
numImagesToDelete: config.DefaultNumImagesToDeletePerCycle,
imageCleanupTimeInterval: config.DefaultImageCleanupTimeInterval,
Expand Down Expand Up @@ -442,8 +442,8 @@ func TestGetCandidateImagesForDeletionImageNoImageState(t *testing.T) {
client := mock_dockerapi.NewMockDockerClient(ctrl)

imageManager := &dockerImageManager{
client: client,
state: dockerstate.NewTaskEngineState(),
client: client,
state: dockerstate.NewTaskEngineState(),
minimumAgeBeforeDeletion: config.DefaultImageDeletionAge,
numImagesToDelete: config.DefaultNumImagesToDeletePerCycle,
imageCleanupTimeInterval: config.DefaultImageCleanupTimeInterval,
Expand All @@ -462,8 +462,8 @@ func TestGetCandidateImagesForDeletionImageJustPulled(t *testing.T) {
client := mock_dockerapi.NewMockDockerClient(ctrl)

imageManager := &dockerImageManager{
client: client,
state: dockerstate.NewTaskEngineState(),
client: client,
state: dockerstate.NewTaskEngineState(),
minimumAgeBeforeDeletion: config.DefaultImageDeletionAge,
numImagesToDelete: config.DefaultNumImagesToDeletePerCycle,
imageCleanupTimeInterval: config.DefaultImageCleanupTimeInterval,
Expand All @@ -487,8 +487,8 @@ func TestGetCandidateImagesForDeletionImageHasContainerReference(t *testing.T) {
client := mock_dockerapi.NewMockDockerClient(ctrl)

imageManager := &dockerImageManager{
client: client,
state: dockerstate.NewTaskEngineState(),
client: client,
state: dockerstate.NewTaskEngineState(),
minimumAgeBeforeDeletion: config.DefaultImageDeletionAge,
numImagesToDelete: config.DefaultNumImagesToDeletePerCycle,
imageCleanupTimeInterval: config.DefaultImageCleanupTimeInterval,
Expand Down Expand Up @@ -528,8 +528,8 @@ func TestGetCandidateImagesForDeletionImageHasMoreContainerReferences(t *testing
client := mock_dockerapi.NewMockDockerClient(ctrl)

imageManager := &dockerImageManager{
client: client,
state: dockerstate.NewTaskEngineState(),
client: client,
state: dockerstate.NewTaskEngineState(),
minimumAgeBeforeDeletion: config.DefaultImageDeletionAge,
numImagesToDelete: config.DefaultNumImagesToDeletePerCycle,
imageCleanupTimeInterval: config.DefaultImageCleanupTimeInterval,
Expand Down Expand Up @@ -606,8 +606,8 @@ func TestImageCleanupExclusionListWithSingleName(t *testing.T) {
PulledAt: time.Now().AddDate(0, -2, 0),
}
imageManager := &dockerImageManager{
client: client,
state: dockerstate.NewTaskEngineState(),
client: client,
state: dockerstate.NewTaskEngineState(),
minimumAgeBeforeDeletion: config.DefaultImageDeletionAge,
numImagesToDelete: config.DefaultNumImagesToDeletePerCycle,
imageCleanupTimeInterval: config.DefaultImageCleanupTimeInterval,
Expand Down Expand Up @@ -665,8 +665,8 @@ func TestImageCleanupExclusionListWithMultipleNames(t *testing.T) {
PulledAt: time.Now().AddDate(0, -2, 0),
}
imageManager := &dockerImageManager{
client: client,
state: dockerstate.NewTaskEngineState(),
client: client,
state: dockerstate.NewTaskEngineState(),
minimumAgeBeforeDeletion: config.DefaultImageDeletionAge,
numImagesToDelete: config.DefaultNumImagesToDeletePerCycle,
imageCleanupTimeInterval: config.DefaultImageCleanupTimeInterval,
Expand Down Expand Up @@ -731,8 +731,8 @@ func TestGetLeastRecentlyUsedImagesLessThanFive(t *testing.T) {
client := mock_dockerapi.NewMockDockerClient(ctrl)

imageManager := &dockerImageManager{
client: client,
state: dockerstate.NewTaskEngineState(),
client: client,
state: dockerstate.NewTaskEngineState(),
minimumAgeBeforeDeletion: config.DefaultImageDeletionAge,
numImagesToDelete: config.DefaultNumImagesToDeletePerCycle,
imageCleanupTimeInterval: config.DefaultImageCleanupTimeInterval,
Expand Down Expand Up @@ -765,8 +765,8 @@ func TestRemoveAlreadyExistingImageNameWithDifferentID(t *testing.T) {
client := mock_dockerapi.NewMockDockerClient(ctrl)

imageManager := &dockerImageManager{
client: client,
state: dockerstate.NewTaskEngineState(),
client: client,
state: dockerstate.NewTaskEngineState(),
minimumAgeBeforeDeletion: config.DefaultImageDeletionAge,
numImagesToDelete: config.DefaultNumImagesToDeletePerCycle,
imageCleanupTimeInterval: config.DefaultImageCleanupTimeInterval,
Expand Down Expand Up @@ -816,8 +816,8 @@ func TestImageCleanupHappyPath(t *testing.T) {
client := mock_dockerapi.NewMockDockerClient(ctrl)

imageManager := &dockerImageManager{
client: client,
state: dockerstate.NewTaskEngineState(),
client: client,
state: dockerstate.NewTaskEngineState(),
minimumAgeBeforeDeletion: 1 * time.Millisecond,
numImagesToDelete: config.DefaultNumImagesToDeletePerCycle,
imageCleanupTimeInterval: config.DefaultImageCleanupTimeInterval,
Expand Down Expand Up @@ -873,8 +873,8 @@ func TestImageCleanupCannotRemoveImage(t *testing.T) {
client := mock_dockerapi.NewMockDockerClient(ctrl)

imageManager := &dockerImageManager{
client: client,
state: dockerstate.NewTaskEngineState(),
client: client,
state: dockerstate.NewTaskEngineState(),
minimumAgeBeforeDeletion: config.DefaultImageDeletionAge,
numImagesToDelete: config.DefaultNumImagesToDeletePerCycle,
imageCleanupTimeInterval: config.DefaultImageCleanupTimeInterval,
Expand Down Expand Up @@ -931,8 +931,8 @@ func TestImageCleanupRemoveImageById(t *testing.T) {
client := mock_dockerapi.NewMockDockerClient(ctrl)

imageManager := &dockerImageManager{
client: client,
state: dockerstate.NewTaskEngineState(),
client: client,
state: dockerstate.NewTaskEngineState(),
minimumAgeBeforeDeletion: config.DefaultImageDeletionAge,
numImagesToDelete: config.DefaultNumImagesToDeletePerCycle,
imageCleanupTimeInterval: config.DefaultImageCleanupTimeInterval,
Expand Down Expand Up @@ -984,8 +984,8 @@ func TestNonECSImageAndContainersCleanupRemoveImage(t *testing.T) {
defer ctrl.Finish()
client := mock_dockerapi.NewMockDockerClient(ctrl)
imageManager := &dockerImageManager{
client: client,
state: dockerstate.NewTaskEngineState(),
client: client,
state: dockerstate.NewTaskEngineState(),
minimumAgeBeforeDeletion: config.DefaultImageDeletionAge,
numImagesToDelete: config.DefaultNumImagesToDeletePerCycle,
imageCleanupTimeInterval: config.DefaultImageCleanupTimeInterval,
Expand Down Expand Up @@ -1220,8 +1220,8 @@ func TestConcurrentRemoveUnusedImages(t *testing.T) {
client := mock_dockerapi.NewMockDockerClient(ctrl)

imageManager := &dockerImageManager{
client: client,
state: dockerstate.NewTaskEngineState(),
client: client,
state: dockerstate.NewTaskEngineState(),
minimumAgeBeforeDeletion: config.DefaultImageDeletionAge,
numImagesToDelete: config.DefaultNumImagesToDeletePerCycle,
imageCleanupTimeInterval: config.DefaultImageCleanupTimeInterval,
Expand Down
Loading

0 comments on commit 3bc095b

Please sign in to comment.