Merge pull request cri-o#6925 from haircommander/no-lock-pid

openshift-merge-robot · web-flow · commit b8764202e9ef · 2023-06-06T18:27:43.000-04:00
server: do not take lock to populate verbose information for containe…
diff --git a/server/container_status.go b/server/container_status.go
@@ -143,7 +143,7 @@ func (s *Server) createContainerInfo(container *oci.Container) (map[string]strin
 	bytes, err := func(metadata *storage.RuntimeContainerMetadata) ([]byte, error) {
 		localContainerInfo := containerInfo{
 			SandboxID:   container.Sandbox(),
-			Pid:         container.State().Pid,
+			Pid:         container.StateNoLock().InitPid,
 			RuntimeSpec: container.Spec(),
 			Privileged:  metadata.Privileged,
 		}
diff --git a/server/inspect.go b/server/inspect.go
@@ -69,7 +69,7 @@ func (s *Server) getContainerInfo(ctx context.Context, id string, getContainerFu
 		isInfra = true
 	}
 	// TODO(mrunalp): should we call UpdateStatus()?
-	ctrState := ctr.State()
+	ctrState := ctr.StateNoLock()
 	if ctrState == nil {
 		return types.ContainerInfo{}, errCtrStateNil
 	}
@@ -79,7 +79,7 @@ func (s *Server) getContainerInfo(ctx context.Context, id string, getContainerFu
 		return types.ContainerInfo{}, errSandboxNotFound
 	}
 
-	pidToReturn := ctrState.Pid
+	pidToReturn := ctrState.InitPid
 	if isInfra && pidToReturn == 0 {
 		// It is possible the infra container doesn't report a PID.
 		// That can either happen if we're using a vm based runtime,
diff --git a/server/inspect_test.go b/server/inspect_test.go
@@ -56,11 +56,9 @@ func TestGetContainerInfo(t *testing.T) {
 		}
 		container.SetMountPoint("/var/foo/container")
 		cstate := &oci.ContainerState{}
-		cstate.State = specs.State{
-			Pid: 42,
-		}
+		cstate.State = specs.State{}
 		cstate.Created = created
-		container.SetState(cstate)
+		container.SetStateAndSpoofPid(cstate)
 		return container
 	}
 	getInfraContainerFunc := func(ctx context.Context, id string) *oci.Container {
@@ -78,8 +76,8 @@ func TestGetContainerInfo(t *testing.T) {
 	if ci.CreatedTime != created.UnixNano() {
 		t.Fatalf("expected same created time %d, got %d", created.UnixNano(), ci.CreatedTime)
 	}
-	if ci.Pid != 42 {
-		t.Fatalf("expected pid 42, got %v", ci.Pid)
+	if ci.Pid != 1 {
+		t.Fatalf("expected pid 1, got %v", ci.Pid)
 	}
 	if ci.Name != "testname" {
 		t.Fatalf("expected name testname, got %s", ci.Name)

Original file line number	Diff line number	Diff line change
`@@ -143,7 +143,7 @@ func (s Server) createContainerInfo(container oci.Container) (map[string]strin`
`143`	`143`	`bytes, err := func(metadata *storage.RuntimeContainerMetadata) ([]byte, error) {`
`144`	`144`	`localContainerInfo := containerInfo{`
`145`	`145`	`SandboxID: container.Sandbox(),`
`146`		`- Pid: container.State().Pid,`
	`146`	`+ Pid: container.StateNoLock().InitPid,`
`147`	`147`	`RuntimeSpec: container.Spec(),`
`148`	`148`	`Privileged: metadata.Privileged,`
`149`	`149`	`}`
Original file line number	Diff line number	Diff line change
`@@ -69,7 +69,7 @@ func (s *Server) getContainerInfo(ctx context.Context, id string, getContainerFu`
`69`	`69`	`isInfra = true`
`70`	`70`	`}`
`71`	`71`	`// TODO(mrunalp): should we call UpdateStatus()?`
`72`		`- ctrState := ctr.State()`
	`72`	`+ ctrState := ctr.StateNoLock()`
`73`	`73`	`if ctrState == nil {`
`74`	`74`	`return types.ContainerInfo{}, errCtrStateNil`
`75`	`75`	`}`
`@@ -79,7 +79,7 @@ func (s *Server) getContainerInfo(ctx context.Context, id string, getContainerFu`
`79`	`79`	`return types.ContainerInfo{}, errSandboxNotFound`
`80`	`80`	`}`
`81`	`81`
`82`		`- pidToReturn := ctrState.Pid`
	`82`	`+ pidToReturn := ctrState.InitPid`
`83`	`83`	`if isInfra && pidToReturn == 0 {`
`84`	`84`	`// It is possible the infra container doesn't report a PID.`
`85`	`85`	`// That can either happen if we're using a vm based runtime,`
Original file line number	Diff line number	Diff line change
`@@ -56,11 +56,9 @@ func TestGetContainerInfo(t *testing.T) {`
`56`	`56`	`}`
`57`	`57`	`container.SetMountPoint("/var/foo/container")`
`58`	`58`	`cstate := &oci.ContainerState{}`
`59`		`- cstate.State = specs.State{`
`60`		`- Pid: 42,`
`61`		`- }`
	`59`	`+ cstate.State = specs.State{}`
`62`	`60`	`cstate.Created = created`
`63`		`- container.SetState(cstate)`
	`61`	`+ container.SetStateAndSpoofPid(cstate)`
`64`	`62`	`return container`
`65`	`63`	`}`
`66`	`64`	`getInfraContainerFunc := func(ctx context.Context, id string) *oci.Container {`
`@@ -78,8 +76,8 @@ func TestGetContainerInfo(t *testing.T) {`
`78`	`76`	`if ci.CreatedTime != created.UnixNano() {`
`79`	`77`	`t.Fatalf("expected same created time %d, got %d", created.UnixNano(), ci.CreatedTime)`
`80`	`78`	`}`
`81`		`- if ci.Pid != 42 {`
`82`		`- t.Fatalf("expected pid 42, got %v", ci.Pid)`
	`79`	`+ if ci.Pid != 1 {`
	`80`	`+ t.Fatalf("expected pid 1, got %v", ci.Pid)`
`83`	`81`	`}`
`84`	`82`	`if ci.Name != "testname" {`
`85`	`83`	`t.Fatalf("expected name testname, got %s", ci.Name)`