Skip to content

Commit

Permalink
PR: comments, bug, cleanup order
Browse files Browse the repository at this point in the history 
Fixed bug with calling `*hcsv2.Host.GetContainer` instead of
`*hcsv2.Host.GetCreatedContainer`.

Removed left over comments, added clarifying comments to
`assertNumberContainers` and `listContaienrStates` interactions.

Reordered namespace and rootfs cleanup.

Signed-off-by: Hamza El-Saawy <hamzaelsaawy@microsoft.com>
  • Loading branch information
@helsaawy
helsaawy committed May 27, 2022
1 parent 39313fa commit fdba25d
Show file tree
Hide file tree
Showing 6 changed files with 22 additions and 39 deletions.
1 change: 0 additions & 1 deletion test/gcs/container_bench_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,6 @@ func BenchmarkContainerExec(b *testing.B) {
b.ResetTimer()
for i := 0; i < b.N; i++ {
ps := testoci.CreateLinuxSpec(ctx, b, id,
// oci.WithTTY,
oci.WithDefaultPathEnv,
oci.WithProcessArgs("/bin/sh", "-c", "true"),
).Process
Expand Down
7 changes: 3 additions & 4 deletions test/gcs/container_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ func TestContainerCreate(t *testing.T) {

assertNumberContainers(ctx, t, rtime, 1)
css := listContainerStates(ctx, t, rtime)
// guaranteed by assertNumberContainers that css will only have 1 element
cs := css[0]
if cs.ID != id {
t.Fatalf("got id %q, wanted %q", cs.ID, id)
Expand Down Expand Up @@ -76,10 +77,9 @@ func TestContainerDelete(t *testing.T) {

cleanupContainer(ctx, t, host, c)

// getContainer will Fatal
_, err := host.GetContainer(id)
_, err := host.GetCreatedContainer(id)
if hr, herr := gcserr.GetHresult(err); herr != nil || hr != gcserr.HrVmcomputeSystemNotFound {
t.Fatalf("GetContainer returned %v, wanted %v", err, gcserr.HrVmcomputeSystemNotFound)
t.Fatalf("GetCreatedContainer returned %v, wanted %v", err, gcserr.HrVmcomputeSystemNotFound)
}
assertNumberContainers(ctx, t, rtime, 0)
}
Expand Down Expand Up @@ -140,7 +140,6 @@ func TestContainerIO(t *testing.T) {
})

c := createStandaloneContainer(ctx, t, host, id,
// oci.WithTTY,
oci.WithProcessArgs(tt.args...),
)
t.Cleanup(func() {
Expand Down
4 changes: 2 additions & 2 deletions test/gcs/cri_bench_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ func BenchmarkCRISanboxCreate(b *testing.B) {
// so kill container to end those and avoid future perf hits
killContainer(ctx, b, c)
cleanupContainer(ctx, b, host, c)
unmountRootfs(ctx, b, scratch)
removeNamespace(ctx, b, nns)
unmountRootfs(ctx, b, scratch)
}
}

Expand Down Expand Up @@ -71,8 +71,8 @@ func BenchmarkCRISandboxStart(b *testing.B) {
killContainer(ctx, b, c)
waitContainer(ctx, b, c, p, true)
cleanupContainer(ctx, b, host, c)
unmountRootfs(ctx, b, scratch)
removeNamespace(ctx, b, nns)
unmountRootfs(ctx, b, scratch)
}
}

Expand Down
15 changes: 3 additions & 12 deletions test/gcs/helper_conn_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ type fakeIO struct {
}

func createStdIO(ctx context.Context, t testing.TB, con stdio.ConnectionSettings) *fakeIO {
// (stdin io.WriteCloser, stdout io.ReadCloser, stderr io.ReadCloser) {
f := &fakeIO{}
if con.StdIn != nil {
f.stdin = newFakeSocket(ctx, t, *con.StdIn, "stdin")
Expand Down Expand Up @@ -120,10 +119,9 @@ func (f *fakeIO) ReadAllErr(ctx context.Context, t testing.TB) string {
}

type fakeSocket struct {
id uint32
n string
ch chan struct{} // closed when dialed (via getFakeSocket)
// m sync.RWMutex
id uint32
n string
ch chan struct{} // closed when dialed (via getFakeSocket)
r, w *os.File
}

Expand Down Expand Up @@ -155,7 +153,6 @@ func newFakeSocket(_ context.Context, t testing.TB, id uint32, n string) *fakeSo
}

func getFakeSocket(id uint32) (*fakeSocket, error) {
// logrus.Debugf("getting fake socket %d", id)
f, ok := _pipes.Load(id)
if !ok {
return nil, unix.ENOENT
Expand All @@ -172,19 +169,16 @@ func getFakeSocket(id uint32) (*fakeSocket, error) {
}

func (s *fakeSocket) Read(b []byte) (int, error) {
// logrus.Debugf("reading from fake socket %d", s.id)
<-s.ch
return s.r.Read(b)
}

func (s *fakeSocket) Write(b []byte) (int, error) {
// logrus.Debugf("writing to fake socket %d", s.id)
<-s.ch
return s.w.Write(b)
}

func (s *fakeSocket) Close() (err error) {
// logrus.Debugf("closing fake socket %d", s.id)
if _, ok := _pipes.LoadAndDelete(s.id); ok {
return nil
}
Expand Down Expand Up @@ -272,7 +266,6 @@ func TestFakeSocket(t *testing.T) {

// host
f := createStdIO(ctx, t, con)
// t.Logf("got std io %v %v %v", stdin, stdout, stderr)

var err error
go func() { // guest
Expand All @@ -286,7 +279,6 @@ func TestFakeSocket(t *testing.T) {
return
}
defer cin.Close()
// t.Logf("dialed conn %#+v", cin)

cout, err = tpt.Dial(*con.StdOut)
if err != nil {
Expand All @@ -295,7 +287,6 @@ func TestFakeSocket(t *testing.T) {
return
}
defer cout.Close()
// t.Logf("dialed conn %#+v", cout)

close(chs)
var b []byte
Expand Down
14 changes: 7 additions & 7 deletions test/gcs/helper_container_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ func createStandaloneContainer(ctx context.Context, t testing.TB, host *hcsv2.Ho

t.Cleanup(func() {
unmountRootfs(ctx, t, scratch)
// hcsv2.RemoveNetworkNamespace(ctx, id)
})

return createContainer(ctx, t, host, id, r)
Expand All @@ -67,7 +66,7 @@ func createContainer(ctx context.Context, t testing.TB, host *hcsv2.Host, id str
}

func getContainer(_ context.Context, t testing.TB, host *hcsv2.Host, id string) *hcsv2.Container {
c, err := host.GetContainer(id)
c, err := host.GetCreatedContainer(id)
if err != nil {
t.Helper()
t.Fatalf("could not get container %q: %v", id, err)
Expand Down Expand Up @@ -196,6 +195,7 @@ func listContainerStates(_ context.Context, t testing.TB, rt runtime.Runtime) []
return css
}

// assertNumberContainers asserts that n containers are found, and then returns the container states.
func assertNumberContainers(ctx context.Context, t testing.TB, rt runtime.Runtime, n int) {
fmt := "found %d running containers, wanted %d"
css := listContainerStates(ctx, t, rt)
Expand Down Expand Up @@ -227,7 +227,7 @@ func getContainerState(ctx context.Context, t testing.TB, rt runtime.Runtime, id

t.Helper()
t.Fatalf("could not find container %q", id)
return runtime.ContainerState{} // jus to make the linter happy
return runtime.ContainerState{} // just to make the linter happy
}

func assertContainerState(ctx context.Context, t testing.TB, rt runtime.Runtime, id, state string) {
Expand Down Expand Up @@ -283,8 +283,8 @@ func createNamespace(ctx context.Context, t testing.TB, nns string) {
}

func removeNamespace(ctx context.Context, t testing.TB, nns string) {
// if err := hcsv2.RemoveNetworkNamespace(ctx, nns); err != nil {
// t.Helper()
// t.Fatalf("could not remove namespace %q: %v", nns, err)
// }
if err := hcsv2.RemoveNetworkNamespace(ctx, nns); err != nil {
t.Helper()
t.Fatalf("could not remove namespace %q: %v", nns, err)
}
}
20 changes: 7 additions & 13 deletions test/gcs/main_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,17 @@ const (
featureStandalone = "StandAlone"
)

var _securityPolicy string

var allFeatures = []string{
featureCRI,
featureStandalone,
}

// flags
var (
flagSecurityPolicy string
flagFeatures = testflag.NewFeatureFlag(allFeatures)
flagJoinGCSCgroup = flag.Bool(
flagFeatures = testflag.NewFeatureFlag(allFeatures)
flagJoinGCSCgroup = flag.Bool(
"join-gcs-cgroup",
false,
"If true, join the same cgroup as the gcs daemon, `/gcs`",
Expand All @@ -57,19 +58,13 @@ var (
)

func init() {
var err error
p := securitypolicy.NewOpenDoorPolicy()
pStr, err := p.EncodeToString()
_securityPolicy, err = p.EncodeToString()
if err != nil {
// really should not get here ...
log.Fatal("could not encode open door policy to string: %w", err)
}

flag.StringVar(
&flagSecurityPolicy,
"security-policy",
pStr,
"The base64-encoded security policy to use during testing",
)
}

func TestMain(m *testing.M) {
Expand All @@ -84,7 +79,6 @@ func TestMain(m *testing.M) {

func setup() (err error) {
os.MkdirAll(guestpath.LCOWRootPrefixInUVM, 0755)
// os.MkdirAll(sockDir, 0755)

if vf := flag.Lookup("test.v"); vf != nil {
if vf.Value.String() == strconv.FormatBool(true) {
Expand Down Expand Up @@ -142,7 +136,7 @@ func getHost(_ context.Context, t testing.TB, rt runtime.Runtime) *hcsv2.Host {

func _getHost(rt runtime.Runtime, tp transport.Transport) (*hcsv2.Host, error) {
h := hcsv2.NewHost(rt, tp)
if err := h.SetSecurityPolicy(flagSecurityPolicy); err != nil {
if err := h.SetSecurityPolicy(_securityPolicy); err != nil {
return nil, fmt.Errorf("could not set host security policy: %w", err)
}

Expand Down

0 comments on commit fdba25d

Please sign in to comment.