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

Remove build \!remote flags from test phase 2 #8379

Merged
merged 2 commits into from
Nov 18, 2020
Merged

Remove build \!remote flags from test phase 2 #8379

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
72b6771
podman-remote network rm --force is broken
rhatdan Nov 17, 2020
3b6d7a3
Remove build \!remote flags from test phase 2
rhatdan Nov 17, 2020
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
37 changes: 28 additions & 9 deletions pkg/api/handlers/compat/networks.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,25 +289,44 @@ func CreateNetwork(w http.ResponseWriter, r *http.Request) {

func RemoveNetwork(w http.ResponseWriter, r *http.Request) {
runtime := r.Context().Value("runtime").(*libpod.Runtime)
config, err := runtime.GetConfig()
if err != nil {
utils.InternalServerError(w, err)
ic := abi.ContainerEngine{Libpod: runtime}

query := struct {
Force bool `schema:"force"`
QiWang19 marked this conversation as resolved.
Show resolved Hide resolved
}{
// This is where you can override the golang default value for one of fields
}

decoder := r.Context().Value("decoder").(*schema.Decoder)
if err := decoder.Decode(&query, r.URL.Query()); err != nil {
utils.Error(w, "Something went wrong.", http.StatusBadRequest, errors.Wrapf(err, "failed to parse parameters for %s", r.URL.String()))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not going to block just because of this... but if you need to resubmit for any reason, could there maybe be a better diagnostic than "Something went wrong"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, although this message is in all over the code. I think this is what Docker did when there was an unexpected error.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@edsantiago I thoroughly concur and have been dinging those as I encounter them. It makes me grind my teeth every time I spot it.

return
}

options := entities.NetworkRmOptions{
Force: query.Force,
}

name := utils.GetName(r)
exists, err := network.Exists(config, name)
reports, err := ic.NetworkRm(r.Context(), []string{name}, options)
if err != nil {
utils.InternalServerError(w, err)
utils.Error(w, "remove Network failed", http.StatusInternalServerError, err)
return
}
if !exists {
utils.Error(w, "network not found", http.StatusNotFound, define.ErrNoSuchNetwork)
if len(reports) == 0 {
utils.Error(w, "remove Network failed", http.StatusInternalServerError, errors.Errorf("internal error"))
return
}
if err := network.RemoveNetwork(config, name); err != nil {
utils.InternalServerError(w, err)
report := reports[0]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should do a length check to make sure it is at least len(1) so we don't panic

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

if report.Err != nil {
if errors.Cause(report.Err) == define.ErrNoSuchNetwork {
utils.Error(w, "network not found", http.StatusNotFound, define.ErrNoSuchNetwork)
return
}
utils.InternalServerError(w, report.Err)
return
}

utils.WriteResponse(w, http.StatusNoContent, "")
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/domain/infra/abi/network.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ func (ic *ContainerEngine) NetworkRm(ctx context.Context, namesOrIds []string, o
if err := ic.Libpod.RemovePod(ctx, pod, true, true); err != nil {
return reports, err
}
} else if err := ic.Libpod.RemoveContainer(ctx, c, true, true); err != nil {
} else if err := ic.Libpod.RemoveContainer(ctx, c, true, true); err != nil && errors.Cause(err) != define.ErrNoSuchCtr {
return reports, err
}
}
Expand Down
3 changes: 1 addition & 2 deletions test/e2e/mount_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
// +build !remote

package integration

import (
Expand All @@ -18,6 +16,7 @@ var _ = Describe("Podman mount", func() {
)

BeforeEach(func() {
SkipIfRemote("Podman mount not supported for remote connections")
SkipIfRootless("Podman mount requires podman unshare first to work")
tempdir, err = CreateTempDirInTempDir()
if err != nil {
Expand Down
11 changes: 9 additions & 2 deletions test/e2e/push_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
// +build !remote

package integration

import (
Expand Down Expand Up @@ -39,6 +37,7 @@ var _ = Describe("Podman push", func() {
})

It("podman push to containers/storage", func() {
SkipIfRemote("Remote push does not support containers-storage transport")
session := podmanTest.Podman([]string{"push", ALPINE, "containers-storage:busybox:test"})
session.WaitWithDefaultTimeout()
Expect(session.ExitCode()).To(Equal(0))
Expand All @@ -49,6 +48,7 @@ var _ = Describe("Podman push", func() {
})

It("podman push to dir", func() {
SkipIfRemote("Remote push does not support dir transport")
bbdir := filepath.Join(podmanTest.TempDir, "busybox")
session := podmanTest.Podman([]string{"push", "--remove-signatures", ALPINE,
fmt.Sprintf("dir:%s", bbdir)})
Expand All @@ -57,6 +57,7 @@ var _ = Describe("Podman push", func() {
})

It("podman push to local registry", func() {
SkipIfRemote("FIXME: This should work")
if podmanTest.Host.Arch == "ppc64le" {
Skip("No registry image for ppc64le")
}
Expand Down Expand Up @@ -87,6 +88,7 @@ var _ = Describe("Podman push", func() {
})

It("podman push to local registry with authorization", func() {
SkipIfRemote("FIXME: This does not seem to be returning an error")
SkipIfRootless("FIXME: Creating content in certs.d we use directories in homedir")
if podmanTest.Host.Arch == "ppc64le" {
Skip("No registry image for ppc64le")
Expand Down Expand Up @@ -163,6 +165,7 @@ var _ = Describe("Podman push", func() {
})

It("podman push to docker-archive", func() {
SkipIfRemote("Remote push does not support docker-archive transport")
tarfn := filepath.Join(podmanTest.TempDir, "alp.tar")
session := podmanTest.Podman([]string{"push", ALPINE,
fmt.Sprintf("docker-archive:%s:latest", tarfn)})
Expand All @@ -171,6 +174,7 @@ var _ = Describe("Podman push", func() {
})

It("podman push to docker daemon", func() {
SkipIfRemote("Remote push does not support docker-daemon transport")
setup := SystemExec("bash", []string{"-c", "systemctl status docker 2>&1"})

if setup.LineInOutputContains("Active: inactive") {
Expand All @@ -196,6 +200,7 @@ var _ = Describe("Podman push", func() {
})

It("podman push to oci-archive", func() {
SkipIfRemote("Remote push does not support oci-archive transport")
tarfn := filepath.Join(podmanTest.TempDir, "alp.tar")
session := podmanTest.Podman([]string{"push", ALPINE,
fmt.Sprintf("oci-archive:%s:latest", tarfn)})
Expand All @@ -204,6 +209,7 @@ var _ = Describe("Podman push", func() {
})

It("podman push to docker-archive no reference", func() {
SkipIfRemote("Remote push does not support docker-archive transport")
tarfn := filepath.Join(podmanTest.TempDir, "alp.tar")
session := podmanTest.Podman([]string{"push", ALPINE,
fmt.Sprintf("docker-archive:%s", tarfn)})
Expand All @@ -212,6 +218,7 @@ var _ = Describe("Podman push", func() {
})

It("podman push to oci-archive no reference", func() {
SkipIfRemote("Remote push does not support oci-archive transport")
ociarc := filepath.Join(podmanTest.TempDir, "alp-oci")
session := podmanTest.Podman([]string{"push", ALPINE,
fmt.Sprintf("oci-archive:%s", ociarc)})
Expand Down
2 changes: 0 additions & 2 deletions test/e2e/run_cgroup_parent_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
// +build !remote

package integration

import (
Expand Down
4 changes: 2 additions & 2 deletions test/e2e/run_signal_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
// +build !remote

package integration

import (
Expand Down Expand Up @@ -46,6 +44,7 @@ var _ = Describe("Podman run with --sig-proxy", func() {
})

Specify("signals are forwarded to container using sig-proxy", func() {
SkipIfRemote("FIXME: This looks like it is supposed to work in remote")
if podmanTest.Host.Arch == "ppc64le" {
Skip("Doesn't work on ppc64le")
}
Expand Down Expand Up @@ -111,6 +110,7 @@ var _ = Describe("Podman run with --sig-proxy", func() {
})

Specify("signals are not forwarded to container with sig-proxy false", func() {
SkipIfRemote("FIXME: This looks like it is supposed to work in remote")
signal := syscall.SIGFPE
if rootless.IsRootless() {
podmanTest.RestoreArtifact(fedoraMinimal)
Expand Down
3 changes: 1 addition & 2 deletions test/e2e/trust_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
// +build !remote

package integration

import (
Expand All @@ -21,6 +19,7 @@ var _ = Describe("Podman trust", func() {
)

BeforeEach(func() {
SkipIfRemote("podman-remote does not support image trust")
tempdir, err = CreateTempDirInTempDir()
if err != nil {
os.Exit(1)
Expand Down