Skip to content

Commit

Permalink
api: Rename Status fields in Container with Unsafe prefix
Browse files Browse the repository at this point in the history
  • Loading branch information
aaithal committed Apr 10, 2017
1 parent 08f3f99 commit bfa0954
Show file tree
Hide file tree
Showing 15 changed files with 143 additions and 143 deletions.
46 changes: 23 additions & 23 deletions agent/api/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,28 +62,28 @@ type Container struct {
DockerConfig DockerConfig `json:"dockerConfig"`
RegistryAuthentication *RegistryAuthenticationData `json:"registryAuthentication"`

// DesiredStatus represents the state where the container should go. Generally,
// DesiredStatusUnsafe represents the state where the container should go. Generally,
// the desired status is informed by the ECS backend as a result of either
// API calls made to ECS or decisions made by the ECS service scheduler,
// though the agent may also set the DesiredStatus if a different "essential"
// though the agent may also set the DesiredStatusUnsafe if a different "essential"
// container in the task exits. The DesiredStatus is almost always either
// ContainerRunning or ContainerStopped.
// NOTE: Do not access DesiredStatus directly. Instead, use `GetDesiredStatus`
// NOTE: Do not access DesiredStatusUnsafe directly. Instead, use `GetDesiredStatus`
// and `SetDesiredStatus`.
// TODO DesiredStatus should probably be private with appropriately written
// TODO DesiredStatusUnsafe should probably be private with appropriately written
// setter/getter. When this is done, we need to ensure that the UnmarshalJSON
// is handled properly so that the state storage continues to work.
DesiredStatus ContainerStatus `json:"desiredStatus"`
desiredStatusLock sync.RWMutex
DesiredStatusUnsafe ContainerStatus `json:"desiredStatus"`
desiredStatusLock sync.RWMutex

// KnownStatus represents the state where the container is.
// NOTE: Do not access `KnownStatus` directly. Instead, use `GetKnownStatus`
// KnownStatusUnsafe represents the state where the container is.
// NOTE: Do not access `KnownStatusUnsafe` directly. Instead, use `GetKnownStatus`
// and `SetKnownStatus`.
// TODO KnownStatus should probably be private with appropriately written
// TODO KnownStatusUnsafe should probably be private with appropriately written
// setter/getter. When this is done, we need to ensure that the UnmarshalJSON
// is handled properly so that the state storage continues to work.
KnownStatus ContainerStatus
knownStatusLock sync.RWMutex
KnownStatusUnsafe ContainerStatus `json:"KnownStatus"`
knownStatusLock sync.RWMutex

// RunDependencies is a list of containers that must be run before
// this one is created
Expand All @@ -100,13 +100,13 @@ type Container struct {
// 'Name: ErrorString' as the 'reason' field.
ApplyingError *DefaultNamedError

// SentStatus represents the last KnownStatus that was sent to the ECS
// SentStatusUnsafe represents the last KnownStatusUnsafe that was sent to the ECS
// SubmitContainerStateChange API.
// TODO SentStatus should probably be private with appropriately written
// TODO SentStatusUnsafe should probably be private with appropriately written
// setter/getter. When this is done, we need to ensure that the UnmarshalJSON is
// handled properly so that the state storage continues to work.
SentStatus ContainerStatus
sentStatusLock sync.RWMutex
SentStatusUnsafe ContainerStatus `json:"SentStatus"`
sentStatusLock sync.RWMutex

KnownExitCode *int
KnownPortBindings []PortBinding
Expand Down Expand Up @@ -159,47 +159,47 @@ func (c *Container) GetKnownStatus() ContainerStatus {
c.knownStatusLock.RLock()
defer c.knownStatusLock.RUnlock()

return c.KnownStatus
return c.KnownStatusUnsafe
}

// SetKnownStatus sets the known status of the container
func (c *Container) SetKnownStatus(status ContainerStatus) {
c.knownStatusLock.Lock()
defer c.knownStatusLock.Unlock()

c.KnownStatus = status
c.KnownStatusUnsafe = status
}

// GetDesiredStatus gets the desired status of the container
func (c *Container) GetDesiredStatus() ContainerStatus {
c.desiredStatusLock.RLock()
defer c.desiredStatusLock.RUnlock()

return c.DesiredStatus
return c.DesiredStatusUnsafe
}

// SetDesiredStatus sets the desired status of the container
func (c *Container) SetDesiredStatus(status ContainerStatus) {
c.desiredStatusLock.Lock()
defer c.desiredStatusLock.Unlock()

c.DesiredStatus = status
c.DesiredStatusUnsafe = status
}

// GetSentStatus safely returns the SentStatus of the container
// GetSentStatus safely returns the SentStatusUnsafe of the container
func (c *Container) GetSentStatus() ContainerStatus {
c.sentStatusLock.RLock()
defer c.sentStatusLock.RUnlock()

return c.SentStatus
return c.SentStatusUnsafe
}

// SetSentStatus safely sets the SentStatus of the container
// SetSentStatus safely sets the SentStatusUnsafe of the container
func (c *Container) SetSentStatus(status ContainerStatus) {
c.sentStatusLock.Lock()
defer c.sentStatusLock.Unlock()

c.SentStatus = status
c.SentStatusUnsafe = status
}

// String returns a human readable string representation of this object
Expand Down
22 changes: 11 additions & 11 deletions agent/api/container_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,17 @@ import (

func TestOverridden(t *testing.T) {
container := &Container{
Name: "name",
Image: "image",
Command: []string{"foo", "bar"},
CPU: 1,
Memory: 1,
Links: []string{},
Ports: []PortBinding{PortBinding{10, 10, "", TransportProtocolTCP}},
Overrides: ContainerOverrides{},
DesiredStatus: ContainerRunning,
AppliedStatus: ContainerRunning,
KnownStatus: ContainerRunning,
Name: "name",
Image: "image",
Command: []string{"foo", "bar"},
CPU: 1,
Memory: 1,
Links: []string{},
Ports: []PortBinding{PortBinding{10, 10, "", TransportProtocolTCP}},
Overrides: ContainerOverrides{},
DesiredStatusUnsafe: ContainerRunning,
AppliedStatus: ContainerRunning,
KnownStatusUnsafe: ContainerRunning,
}

overridden := container.Overridden()
Expand Down
14 changes: 7 additions & 7 deletions agent/api/task.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,13 +157,13 @@ func (task *Task) initializeEmptyVolumes() {
mountPoints[i] = MountPoint{SourceVolume: volume, ContainerPath: containerPath}
}
sourceContainer := &Container{
Name: emptyHostVolumeName,
Image: emptyvolume.Image + ":" + emptyvolume.Tag,
Command: []string{emptyvolume.Command}, // Command required, but this only gets created so N/A
MountPoints: mountPoints,
Essential: false,
IsInternal: true,
DesiredStatus: ContainerRunning,
Name: emptyHostVolumeName,
Image: emptyvolume.Image + ":" + emptyvolume.Tag,
Command: []string{emptyvolume.Command}, // Command required, but this only gets created so N/A
MountPoints: mountPoints,
Essential: false,
IsInternal: true,
DesiredStatusUnsafe: ContainerRunning,
}
task.Containers = append(task.Containers, sourceContainer)
}
Expand Down
28 changes: 14 additions & 14 deletions agent/api/task_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -719,14 +719,14 @@ func TestTaskUpdateKnownStatusHappyPath(t *testing.T) {
KnownStatusUnsafe: TaskStatusNone,
Containers: []*Container{
&Container{
KnownStatus: ContainerCreated,
KnownStatusUnsafe: ContainerCreated,
},
&Container{
KnownStatus: ContainerStopped,
Essential: true,
KnownStatusUnsafe: ContainerStopped,
Essential: true,
},
&Container{
KnownStatus: ContainerRunning,
KnownStatusUnsafe: ContainerRunning,
},
},
}
Expand All @@ -743,15 +743,15 @@ func TestTaskUpdateKnownStatusNotChangeToRunningWithEssentialContainerStopped(t
KnownStatusUnsafe: TaskCreated,
Containers: []*Container{
&Container{
KnownStatus: ContainerRunning,
Essential: true,
KnownStatusUnsafe: ContainerRunning,
Essential: true,
},
&Container{
KnownStatus: ContainerStopped,
Essential: true,
KnownStatusUnsafe: ContainerStopped,
Essential: true,
},
&Container{
KnownStatus: ContainerRunning,
KnownStatusUnsafe: ContainerRunning,
},
},
}
Expand All @@ -768,15 +768,15 @@ func TestTaskUpdateKnownStatusToPendingWithEssentialContainerStopped(t *testing.
KnownStatusUnsafe: TaskStatusNone,
Containers: []*Container{
&Container{
KnownStatus: ContainerCreated,
Essential: true,
KnownStatusUnsafe: ContainerCreated,
Essential: true,
},
&Container{
KnownStatus: ContainerStopped,
Essential: true,
KnownStatusUnsafe: ContainerStopped,
Essential: true,
},
&Container{
KnownStatus: ContainerCreated,
KnownStatusUnsafe: ContainerCreated,
},
},
}
Expand Down
2 changes: 1 addition & 1 deletion agent/api/testutils/container_equal.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func ContainersEqual(lhs, rhs *api.Container) bool {
if !ContainerOverridesEqual(lhs.Overrides, rhs.Overrides) {
return false
}
if lhs.DesiredStatus != rhs.DesiredStatus || lhs.KnownStatus != rhs.KnownStatus {
if lhs.DesiredStatusUnsafe != rhs.DesiredStatusUnsafe || lhs.KnownStatusUnsafe != rhs.KnownStatusUnsafe {
return false
}
if lhs.AppliedStatus != rhs.AppliedStatus {
Expand Down
8 changes: 4 additions & 4 deletions agent/api/testutils/container_equal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@ func TestContainerEqual(t *testing.T) {
Container{EntryPoint: &[]string{"1", "2"}}, Container{EntryPoint: &[]string{"1", "2"}},
Container{Environment: map[string]string{}}, Container{Environment: map[string]string{}},
Container{Environment: map[string]string{"a": "b", "c": "d"}}, Container{Environment: map[string]string{"c": "d", "a": "b"}},
Container{DesiredStatus: ContainerRunning}, Container{DesiredStatus: ContainerRunning},
Container{DesiredStatusUnsafe: ContainerRunning}, Container{DesiredStatusUnsafe: ContainerRunning},
Container{AppliedStatus: ContainerRunning}, Container{AppliedStatus: ContainerRunning},
Container{KnownStatus: ContainerRunning}, Container{KnownStatus: ContainerRunning},
Container{KnownStatusUnsafe: ContainerRunning}, Container{KnownStatusUnsafe: ContainerRunning},
Container{KnownExitCode: nil}, Container{KnownExitCode: nil},
Container{KnownExitCode: onePtr}, Container{KnownExitCode: anotherOnePtr},
}
Expand All @@ -65,9 +65,9 @@ func TestContainerEqual(t *testing.T) {
Container{EntryPoint: &[]string{"1", "2"}}, Container{EntryPoint: &[]string{"2", "1"}},
Container{EntryPoint: &[]string{"1", "2"}}, Container{EntryPoint: &[]string{"1", "二"}},
Container{Environment: map[string]string{"a": "b", "c": "d"}}, Container{Environment: map[string]string{"し": "d", "a": "b"}},
Container{DesiredStatus: ContainerRunning}, Container{DesiredStatus: ContainerStopped},
Container{DesiredStatusUnsafe: ContainerRunning}, Container{DesiredStatusUnsafe: ContainerStopped},
Container{AppliedStatus: ContainerRunning}, Container{AppliedStatus: ContainerStopped},
Container{KnownStatus: ContainerRunning}, Container{KnownStatus: ContainerStopped},
Container{KnownStatusUnsafe: ContainerRunning}, Container{KnownStatusUnsafe: ContainerStopped},
Container{KnownExitCode: nil}, Container{KnownExitCode: onePtr},
Container{KnownExitCode: onePtr}, Container{KnownExitCode: twoPtr},
}
Expand Down
44 changes: 22 additions & 22 deletions agent/engine/dependencygraph/graph_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,18 +30,18 @@ func volumeStrToVol(vols []string) []api.VolumeFrom {

func runningContainer(name string, links, volumes []string) *api.Container {
return &api.Container{
Name: name,
Links: links,
VolumesFrom: volumeStrToVol(volumes),
DesiredStatus: api.ContainerRunning,
Name: name,
Links: links,
VolumesFrom: volumeStrToVol(volumes),
DesiredStatusUnsafe: api.ContainerRunning,
}
}
func createdContainer(name string, links, volumes []string) *api.Container {
return &api.Container{
Name: name,
Links: links,
VolumesFrom: volumeStrToVol(volumes),
DesiredStatus: api.ContainerCreated,
Name: name,
Links: links,
VolumesFrom: volumeStrToVol(volumes),
DesiredStatusUnsafe: api.ContainerCreated,
}
}

Expand All @@ -56,8 +56,8 @@ func TestValidDependencies(t *testing.T) {
task = &api.Task{
Containers: []*api.Container{
&api.Container{
Name: "redis",
DesiredStatus: api.ContainerRunning,
Name: "redis",
DesiredStatusUnsafe: api.ContainerRunning,
},
},
}
Expand Down Expand Up @@ -112,8 +112,8 @@ func TestDependenciesAreResolved(t *testing.T) {
task := &api.Task{
Containers: []*api.Container{
&api.Container{
Name: "redis",
DesiredStatus: api.ContainerRunning,
Name: "redis",
DesiredStatusUnsafe: api.ContainerRunning,
},
},
}
Expand Down Expand Up @@ -148,13 +148,13 @@ func TestDependenciesAreResolved(t *testing.T) {
if !resolved {
t.Error("data volume with no deps should resolve")
}
dbdata.KnownStatus = api.ContainerCreated
dbdata.KnownStatusUnsafe = api.ContainerCreated

resolved = DependenciesAreResolved(php, task.Containers)
if resolved {
t.Error("Php shouldn't run, db is not created")
}
db.KnownStatus = api.ContainerCreated
db.KnownStatusUnsafe = api.ContainerCreated
resolved = DependenciesAreResolved(php, task.Containers)
if resolved {
t.Error("Php shouldn't run, db is not running")
Expand All @@ -164,7 +164,7 @@ func TestDependenciesAreResolved(t *testing.T) {
if !resolved {
t.Error("db should be resolved, dbdata volume is Created")
}
db.KnownStatus = api.ContainerRunning
db.KnownStatusUnsafe = api.ContainerRunning

resolved = DependenciesAreResolved(php, task.Containers)
if !resolved {
Expand All @@ -174,14 +174,14 @@ func TestDependenciesAreResolved(t *testing.T) {

func TestRunningependsOnDependencies(t *testing.T) {
c1 := &api.Container{
Name: "a",
KnownStatus: api.ContainerStatusNone,
Name: "a",
KnownStatusUnsafe: api.ContainerStatusNone,
}
c2 := &api.Container{
Name: "b",
KnownStatus: api.ContainerStatusNone,
DesiredStatus: api.ContainerCreated,
RunDependencies: []string{"a"},
Name: "b",
KnownStatusUnsafe: api.ContainerStatusNone,
DesiredStatusUnsafe: api.ContainerCreated,
RunDependencies: []string{"a"},
}
task := &api.Task{Containers: []*api.Container{c1, c2}}

Expand All @@ -192,7 +192,7 @@ func TestRunningependsOnDependencies(t *testing.T) {
if DependenciesAreResolved(c2, task.Containers) {
t.Error("Dependencies should not be resolved")
}
task.Containers[0].KnownStatus = api.ContainerRunning
task.Containers[0].KnownStatusUnsafe = api.ContainerRunning

if !DependenciesAreResolved(c2, task.Containers) {
t.Error("Dependencies should be resolved")
Expand Down
Loading

0 comments on commit bfa0954

Please sign in to comment.