From cbdd7e94579641c52f47cfd7f19735fff2cce6b0 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Wed, 18 Jun 2025 12:43:22 +0200 Subject: [PATCH] remove unused ShouldRestart() code Deadcode should that the ShouldRestart() API endpoint was never wired into the router so the endpoint did not existed and the bindings called a non existing endpoint which returnd 404 which the binding code assumed means no restart. As such remove all this code as it didn't do anything useful. And IMO exposing a shouldrestart API always feeled wrong to me. The client should not have to deal with this. This commit does not change the behavior but it also does not make an attempt to fix the broken restart handling with the rmeote client. Given we do not seem to have any user reports about this it seems it is not used. Signed-off-by: Paul Holzinger --- pkg/api/handlers/libpod/containers.go | 23 --------------------- pkg/bindings/containers/containers.go | 19 +++--------------- pkg/domain/infra/abi/containers.go | 10 --------- pkg/domain/infra/tunnel/containers.go | 29 ++------------------------- 4 files changed, 5 insertions(+), 76 deletions(-) diff --git a/pkg/api/handlers/libpod/containers.go b/pkg/api/handlers/libpod/containers.go index a7ed9283b3..03cecbb8e7 100644 --- a/pkg/api/handlers/libpod/containers.go +++ b/pkg/api/handlers/libpod/containers.go @@ -471,26 +471,3 @@ func UpdateContainer(w http.ResponseWriter, r *http.Request) { } utils.WriteResponse(w, http.StatusCreated, ctr.ID()) } - -func ShouldRestart(w http.ResponseWriter, r *http.Request) { - runtime := r.Context().Value(api.RuntimeKey).(*libpod.Runtime) - // Now use the ABI implementation to prevent us from having duplicate - // code. - containerEngine := abi.ContainerEngine{Libpod: runtime} - - name := utils.GetName(r) - report, err := containerEngine.ShouldRestart(r.Context(), name) - if err != nil { - if errors.Is(err, define.ErrNoSuchCtr) { - utils.ContainerNotFound(w, name, err) - return - } - utils.InternalServerError(w, err) - return - } - if report.Value { - utils.WriteResponse(w, http.StatusNoContent, "") - } else { - utils.ContainerNotFound(w, name, define.ErrNoSuchCtr) - } -} diff --git a/pkg/bindings/containers/containers.go b/pkg/bindings/containers/containers.go index 5665db2110..b42ce58374 100644 --- a/pkg/bindings/containers/containers.go +++ b/pkg/bindings/containers/containers.go @@ -461,20 +461,7 @@ func ContainerInit(ctx context.Context, nameOrID string, options *InitOptions) e return response.Process(nil) } -func ShouldRestart(ctx context.Context, nameOrID string, options *ShouldRestartOptions) (bool, error) { - if options == nil { - options = new(ShouldRestartOptions) - } - _ = options - conn, err := bindings.GetClient(ctx) - if err != nil { - return false, err - } - response, err := conn.DoRequest(ctx, nil, http.MethodPost, "/containers/%s/shouldrestart", nil, nil, nameOrID) - if err != nil { - return false, err - } - defer response.Body.Close() - - return response.IsSuccess(), nil +// Deprecated: This function always returns false, the server API endpoint never existed. +func ShouldRestart(_ context.Context, _ string, _ *ShouldRestartOptions) (bool, error) { + return false, nil } diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go index e7111dc5ba..5b06d2c59f 100644 --- a/pkg/domain/infra/abi/containers.go +++ b/pkg/domain/infra/abi/containers.go @@ -1671,16 +1671,6 @@ func (ic *ContainerEngine) ContainerStats(ctx context.Context, namesOrIds []stri return statsChan, nil } -// ShouldRestart returns whether the container should be restarted -func (ic *ContainerEngine) ShouldRestart(ctx context.Context, nameOrID string) (*entities.BoolReport, error) { - ctr, err := ic.Libpod.LookupContainer(nameOrID) - if err != nil { - return nil, err - } - - return &entities.BoolReport{Value: ctr.ShouldRestart(ctx)}, nil -} - // ContainerRename renames the given container. func (ic *ContainerEngine) ContainerRename(ctx context.Context, nameOrID string, opts entities.ContainerRenameOptions) error { ctr, err := ic.Libpod.LookupContainer(nameOrID) diff --git a/pkg/domain/infra/tunnel/containers.go b/pkg/domain/infra/tunnel/containers.go index 75b5d405b3..5f57049d6c 100644 --- a/pkg/domain/infra/tunnel/containers.go +++ b/pkg/domain/infra/tunnel/containers.go @@ -800,17 +800,7 @@ func (ic *ContainerEngine) ContainerStart(ctx context.Context, namesOrIds []stri if ctr.AutoRemove { // Defer the removal, so we can return early if needed and // de-spaghetti the code. - defer func() { - shouldRestart, err := containers.ShouldRestart(ic.ClientCtx, ctr.ID, nil) - if err != nil { - logrus.Errorf("Failed to check if %s should restart: %v", ctr.ID, err) - return - } - - if !shouldRestart && ctr.AutoRemove { - removeContainer(ctr.ID, ctr.CIDFile) - } - }() + defer removeContainer(ctr.ID, ctr.CIDFile) } report.ExitCode = code @@ -942,17 +932,7 @@ func (ic *ContainerEngine) ContainerRun(ctx context.Context, opts entities.Conta if opts.Rm { // Defer the removal, so we can return early if needed and // de-spaghetti the code. - defer func() { - shouldRestart, err := containers.ShouldRestart(ic.ClientCtx, con.ID, nil) - if err != nil { - logrus.Errorf("Failed to check if %s should restart: %v", con.ID, err) - return - } - - if !shouldRestart { - _ = removeContainer(con.ID, opts.CIDFile, false) - } - }() + defer removeContainer(con.ID, opts.CIDFile, false) } report.ExitCode = code @@ -1068,11 +1048,6 @@ func (ic *ContainerEngine) ContainerStats(ctx context.Context, namesOrIds []stri return containers.Stats(ic.ClientCtx, namesOrIds, new(containers.StatsOptions).WithStream(options.Stream).WithInterval(options.Interval).WithAll(options.All)) } -// ShouldRestart reports back whether the container will restart. -func (ic *ContainerEngine) ShouldRestart(_ context.Context, id string) (bool, error) { - return containers.ShouldRestart(ic.ClientCtx, id, nil) -} - // ContainerRename renames the given container. func (ic *ContainerEngine) ContainerRename(ctx context.Context, nameOrID string, opts entities.ContainerRenameOptions) error { return containers.Rename(ic.ClientCtx, nameOrID, new(containers.RenameOptions).WithName(opts.NewName))