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

engine: adding poll function during progressTask #1876

Merged
merged 1 commit into from
Feb 22, 2019
Merged

engine: adding poll function during progressTask #1876

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 13 additions & 13 deletions agent/engine/dependencygraph/graph.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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