Skip to content

Commit

Permalink
pod: Make state handling functions set in-memory state.
Browse files Browse the repository at this point in the history 
Previously, setPodState() and setContainerState() only set the on-disk
state of a pod or container respectively. They now also set the
in-memory state. This is cleaner, less surprising, removes the burden
from the caller to manage the in-memory state and also ensures that both
versions of the state are kept in sync (safer).

Fixes containers#279.

Signed-off-by: James O. D. Hunt <james.o.hunt@intel.com>
  • Loading branch information
@jodh-intel
jodh-intel committed Jun 12, 2017
1 parent 90ef63f commit 0e47e70
Show file tree
Hide file tree
Showing 2 changed files with 239 additions and 10 deletions.
64 changes: 54 additions & 10 deletions pod.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -438,13 +438,18 @@ func (p *Pod) GetContainer(containerID string) *Container {
}

func (p *Pod) createSetStates() error {
p.state.State = StateReady
err := p.setPodState(p.state)
podState := State{
State: StateReady,
// retain existing URL value
URL: p.state.URL,
}

err := p.setPodState(podState)
if err != nil {
return err
}

err = p.setContainersState(StateReady)
err = p.setContainersState(podState.State)
if err != nil {
return err
}
Expand Down Expand Up @@ -617,13 +622,18 @@ func (p *Pod) startCheckStates() error {
}

func (p *Pod) startSetStates() error {
p.state.State = StateRunning
err := p.setPodState(p.state)
podState := State{
State: StateRunning,
// retain existing URL value
URL: p.state.URL,
}

err := p.setPodState(podState)
if err != nil {
return err
}

err = p.setContainersState(StateRunning)
err = p.setContainersState(podState.State)
if err != nil {
return err
}
Expand Down Expand Up @@ -742,13 +752,18 @@ func (p *Pod) stopCheckStates() error {
}

func (p *Pod) stopSetStates() error {
err := p.setContainersState(StateStopped)
podState := State{
State: StateStopped,
// retain existing URL value
URL: p.state.URL,
}

err := p.setContainersState(podState.State)
if err != nil {
return err
}

p.state.State = StateStopped
err = p.setPodState(p.state)
err = p.setPodState(podState)
if err != nil {
return err
}
Expand Down Expand Up @@ -824,7 +839,13 @@ func (p *Pod) enter(args []string) error {
return nil
}

// setPodState sets both the in-memory and on-disk state of the
// pod.
func (p *Pod) setPodState(state State) error {
// update in-memory state
p.state = state

// update on-disk state
err := p.storage.storePodResource(p.id, stateFileType, state)
if err != nil {
return err
Expand All @@ -833,6 +854,20 @@ func (p *Pod) setPodState(state State) error {
return nil
}

func (p *Pod) getContainer(containerID string) (*Container, error) {
if containerID == "" {
return &Container{}, errNeedContainerID
}

for _, c := range p.containers {
if c.id == containerID {
return c, nil
}
}

return nil, fmt.Errorf("pod %v has no container with ID %v", p.ID(), containerID)
}

func (p *Pod) setContainerState(containerID string, state stateString) error {
if containerID == "" {
return errNeedContainerID
Expand All @@ -842,7 +877,16 @@ func (p *Pod) setContainerState(containerID string, state stateString) error {
State: state,
}

err := p.storage.storeContainerResource(p.id, containerID, stateFileType, contState)
c, err := p.getContainer(containerID)
if err != nil {
return err
}

// update in-memory state
c.state = contState

// update on-disk state
err = p.storage.storeContainerResource(p.id, containerID, stateFileType, contState)
if err != nil {
return err
}
Expand Down
185 changes: 185 additions & 0 deletions pod_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,142 @@ func TestPodEnterSuccessful(t *testing.T) {
}
}

func TestPodSetPodAndContainerState(t *testing.T) {
contID := "505"
contConfig := newTestContainerConfigNoop(contID)
hConfig := newHypervisorConfig(nil, nil)

// create a pod
p, err := testCreatePod(t, testPodID, MockHypervisor, hConfig, NoopAgentType, NoopNetworkModel, NetworkConfig{}, []ContainerConfig{contConfig}, nil)
if err != nil {
t.Fatal(err)
}

l := len(p.GetAllContainers())
if l != 1 {
t.Fatalf("Expected 1 container found %v", l)
}

initialPodState := State{
State: StateReady,
URL: "",
}

// initially, a container has an empty state
initialContainerState := State{
State: "",
URL: "",
}

if p.state.State != initialPodState.State {
t.Errorf("Expected pod state %v, got %v", initialPodState.State, p.state.State)
}

if p.state.URL != initialPodState.URL {
t.Errorf("Expected pod state URL %v, got %v", initialPodState.URL, p.state.URL)
}

c, err := p.getContainer(contID)
if err != nil {
t.Fatalf("Failed to retrieve container %v: %v", contID, err)
}

if c.state.State != initialContainerState.State {
t.Errorf("Expected container state %v, got %v", initialContainerState.State, c.state.State)
}

if c.state.URL != initialContainerState.URL {
t.Errorf("Expected container state URL %v, got %v", initialContainerState.URL, c.state.URL)
}

// persist to disk
err = p.storePod()
if err != nil {
t.Fatal(err)
}

newPodState := State{
State: StateRunning,
URL: "http://pod/url",
}

newContainerState := State{
State: StateStopped,
URL: "",
}

// force pod state change
err = p.setPodState(newPodState)
if err != nil {
t.Fatalf("Unexpected error: %v (pod %+v)", err, p)
}

// check the in-memory state is correct
if p.state.State != newPodState.State {
t.Errorf("Expected state %v, got %v", newPodState.State, p.state.State)
}

if p.state.URL != newPodState.URL {
t.Errorf("Expected state URL %v, got %v", newPodState.URL, p.state.URL)
}

// force container state change
err = p.setContainerState(contID, newContainerState.State)
if err != nil {
t.Fatalf("Unexpected error: %v (pod %+v)", err, p)
}

// check the in-memory state is correct
if c.state.State != newContainerState.State {
t.Errorf("Expected state %v, got %v", newContainerState.State, c.state.State)
}

if c.state.URL != newContainerState.URL {
t.Errorf("Expected state URL %v, got %v", newContainerState.URL, c.state.URL)
}

// force state to be read from disk
p2, err := fetchPod(p.ID())
if err != nil {
t.Fatalf("Failed to fetch pod %v: %v", p.ID(), err)
}

// check on-disk state is correct
if p2.state.State != newPodState.State {
t.Errorf("Expected state %v, got %v", newPodState.State, p2.state.State)
}

if p2.state.URL != newPodState.URL {
t.Errorf("Expected state URL %v, got %v", newPodState.URL, p2.state.URL)
}

c2, err := p2.getContainer(contID)
if err != nil {
t.Fatalf("Failed to find container %v: %v", contID, err)
}

// check on-disk state is correct
if c2.state.State != newContainerState.State {
t.Errorf("Expected state %v, got %v", newContainerState.State, c2.state.State)
}

if c2.state.URL != newContainerState.URL {
t.Errorf("Expected state URL %v, got %v", newContainerState.URL, c2.state.URL)
}

// revert pod state to allow it to be deleted
err = p.setPodState(initialPodState)
if err != nil {
t.Fatalf("Unexpected error: %v (pod %+v)", err, p)
}

// clean up
err = p.delete()
if err != nil {
t.Fatal(err)
}
}

func TestPodSetPodStateFailingStorePodResource(t *testing.T) {
fs := &filesystem{}
pod := &Pod{
Expand Down Expand Up @@ -804,3 +940,52 @@ func TestSetAnnotations(t *testing.T) {
t.Fatal()
}
}

func TestPodGetContainer(t *testing.T) {

emptyPod := Pod{}
_, err := emptyPod.getContainer("")
if err == nil {
t.Fatal("Expected error for containerless pod")
}

_, err = emptyPod.getContainer("foo")
if err == nil {
t.Fatal("Expected error for containerless pod and invalid containerID")
}

hConfig := newHypervisorConfig(nil, nil)
p, err := testCreatePod(t, testPodID, MockHypervisor, hConfig, NoopAgentType, NoopNetworkModel, NetworkConfig{}, nil, nil)
if err != nil {
t.Fatal(err)
}

p.state.URL = noopProxyURL

contID := "999"
contConfig := newTestContainerConfigNoop(contID)
_, err = createContainer(p, contConfig)
if err != nil {
t.Fatalf("Failed to create container %+v in pod %+v: %v", contConfig, p, err)
}

got := false
for _, c := range p.GetAllContainers() {
c2, err := p.getContainer(c.ID())
if err != nil {
t.Fatalf("Failed to find container %v: %v", c.ID(), err)
}

if c2.ID() != c.ID() {
t.Fatalf("Expected container %v but got %v", c.ID(), c2.ID())
}

if c2.ID() == contID {
got = true
}
}

if !got {
t.Fatalf("Failed to find container %v", contID)
}
}

0 comments on commit 0e47e70

Please sign in to comment.