track podman events in kube play #2
Conversation
vyasgun
force-pushed
the
pr/kube-play-progress
branch
3 times, most recently
from
6220ce5 to
58e21d4
Compare
pkg/domain/infra/abi/play.go
Outdated
| "github.com/containers/podman/v4/pkg/domain/entities" | ||
| v1apps "github.com/containers/podman/v4/pkg/k8s.io/api/apps/v1" | ||
| v1 "github.com/containers/podman/v4/pkg/k8s.io/api/core/v1" | ||
| metav1 "github.com/containers/podman/v4/pkg/k8s.io/apimachinery/pkg/apis/meta/v1" |
There was a problem hiding this comment.
Minor stylistic question: I wonder if it would make sense to match the naming convention of the already-imported v1apps and name this one v1meta?
pkg/domain/infra/abi/play.go
Outdated
| "io" | ||
| "net/http" | ||
| "os" | ||
| "path/filepath" | ||
| "sigs.k8s.io/yaml" | ||
| "strconv" | ||
| "strings" | ||
| "sync" | ||
| "time" |
There was a problem hiding this comment.
I think you may want to ensure your IDE is running goimports along with gofmt on save. Builtin imports I believe are automatically moved to the top imports block.
cmd/podman/kube/play.go
Outdated
| @@ -446,6 +446,7 @@ func teardown(body io.Reader, options entities.PlayKubeDownOptions) error { | |||
| } | |||
|
|
|||
| func kubeplay(body io.Reader) error { | |||
| //go abi.PrintProgress(registry.GetContext(), registry.ContainerEngine(), os.Stdout) | |||
There was a problem hiding this comment.
It seems the --quiet flag is false by default. Perhaps it would be nice to update that flag to silence this progress as well?
pkg/domain/infra/abi/play.go
Outdated
| @@ -46,6 +50,8 @@ const sdNotifyAnnotation = "io.containers.sdnotify" | |||
| // default network created/used by kube | |||
| const kubeDefaultNetwork = "podman-default-kube-network" | |||
|
|
|||
| var eventFiltersCh = make(chan string) | |||
There was a problem hiding this comment.
I would guess that the events channel should be a member of ContainerEngine
pkg/domain/infra/abi/play.go
Outdated
| @@ -181,6 +245,13 @@ func (ic *ContainerEngine) PlayKube(ctx context.Context, body io.Reader, options | |||
| } | |||
| }() | |||
|
|
|||
| if eventFilters, err := getEventFilters(documentList); err != nil { | |||
| return nil, err | |||
| } else { | |||
There was a problem hiding this comment.
Stylistic note: the else statement is probably unnecessary
pkg/domain/infra/abi/play.go
Outdated
| for { | ||
|
|
||
| select { | ||
| case <-ctx.Done(): |
There was a problem hiding this comment.
This might be able to be a single select statement with multiple cases and no default case. Whichever channel returns first, that one's block executes, then the select statement exits and the loop repeats. The break statement would then just be a continue I believe
pkg/api/handlers/libpod/kube.go
Outdated
| go abi.PrintProgress(ctx, &containerEngine, w) | ||
|
|
||
| _, err = containerEngine.PlayKube(ctx, r.Body, options) | ||
| cancel() |
There was a problem hiding this comment.
This might be best as a defer cancel()
vyasgun
force-pushed
the
pr/kube-play-progress
branch
6 times, most recently
from
de1d910 to
b45b6bf
Compare
Signed-off-by: Gunjan Vyas <[email protected]>
vyasgun
force-pushed
the
pr/kube-play-progress
branch
from
b45b6bf to
41d05b9
Compare
|
A friendly reminder that this PR had no activity for 30 days. |
Does this PR introduce a user-facing change?