Skip to content

Commit

Permalink
Ensure containers are cleaned up when SIGINT is called
Browse files Browse the repository at this point in the history
  • Loading branch information
@joshbranham
joshbranham committed Jan 9, 2025
1 parent 52301c1 commit 6731810
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 72 deletions.
103 changes: 50 additions & 53 deletions cmd/ocm-backplane/console/console.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import (
"time"

"github.com/Masterminds/semver"
homedir "github.com/mitchellh/go-homedir"
"github.com/mitchellh/go-homedir"
consolev1typedclient "github.com/openshift/client-go/console/clientset/versioned/typed/console/v1"
consolev1alpha1typedclient "github.com/openshift/client-go/console/clientset/versioned/typed/console/v1alpha1"
operatorv1typedclient "github.com/openshift/client-go/operator/clientset/versioned/typed/operator/v1"
Expand All @@ -56,8 +56,7 @@ type containerEngineInterface interface {
pullImage(imageName string) error
putFileToMount(filename string, content []byte) error
stopContainer(containerName string) error
// some container engines have special handlings for different types of containers
runConsoleContainer(containerName string, port string, consoleArgs []string, envVars []envVar) error
runConsoleContainer(ctx context.Context, cancel context.CancelFunc, containerName string, port string, consoleArgs []string, envVars []envVar) error
runMonitorPlugin(containerName string, consoleContainerName string, nginxConf string, pluginArgs []string) error
containerIsExist(containerName string) (bool, error)
}
Expand Down Expand Up @@ -86,6 +85,8 @@ type consoleOptions struct {
monitorPluginPort string
monitorPluginImage string
terminationFunction execActionOnTermInterface
ctx context.Context
cancel context.CancelFunc
}

// envVar for environment variable passing to container
Expand All @@ -101,7 +102,7 @@ const (
PODMAN = "podman"
// Linux name in runtime.GOOS
LINUX = "linux"
// MacOS name in runtime.GOOS
// MACOS name in runtime.GOOS
MACOS = "darwin"

// Environment variable that indicates if open by browser is set as default
Expand Down Expand Up @@ -132,21 +133,25 @@ const (
var (
validContainerEngines = []string{PODMAN, DOCKER}
// For mocking
createClientSet = func(c *rest.Config) (kubernetes.Interface, error) { return kubernetes.NewForConfig(c) }
createCommand = exec.Command
createClientSet = func(c *rest.Config) (kubernetes.Interface, error) { return kubernetes.NewForConfig(c) }
createCommand = exec.Command
createCommandContext = exec.CommandContext

// Pull Secret saving directory
pullSecretConfigDirectory string

ctx, cancel = context.WithCancel(context.Background())
)

func newConsoleOptions() *consoleOptions {
return &consoleOptions{terminationFunction: &execActionOnTermStruct{}}
func newConsoleOptions(ctx context.Context) *consoleOptions {
opts := &consoleOptions{
terminationFunction: &execActionOnTermStruct{},
}
opts.ctx, opts.cancel = context.WithCancel(ctx)

return opts
}

func NewConsoleCmd() *cobra.Command {
ops := newConsoleOptions()
ops := newConsoleOptions(context.Background())
consoleCmd := &cobra.Command{
Use: "console",
Short: "Launch OpenShift web console for the current cluster",
Expand All @@ -157,7 +162,9 @@ func NewConsoleCmd() *cobra.Command {
You can specify container engine with -c. If not specified, it will lookup the PATH in the order of podman and docker.
`,
SilenceUsage: true,
RunE: ops.run,
RunE: func(cmd *cobra.Command, args []string) error {
return ops.run()
},
}

flags := consoleCmd.Flags()
Expand Down Expand Up @@ -205,7 +212,7 @@ func NewConsoleCmd() *cobra.Command {
return consoleCmd
}

func (o *consoleOptions) run(cmd *cobra.Command, argv []string) error {
func (o *consoleOptions) run() error {
err := o.determineOpenBrowser()
if err != nil {
return err
Expand All @@ -222,7 +229,6 @@ func (o *consoleOptions) run(cmd *cobra.Command, argv []string) error {
if err != nil {
return err
}
// finialize the console image url
err = o.determineImage(kubeconfig)
if err != nil {
return err
Expand Down Expand Up @@ -374,7 +380,6 @@ func (o *consoleOptions) getContainerEngineImpl() (containerEngineInterface, err
} else if runtime.GOOS == LINUX && containerEngine == DOCKER {
containerEngineImpl = &dockerLinux{}
} else if runtime.GOOS == MACOS && containerEngine == DOCKER {
logger.Warnln("Docker on MacOS is not tested")
containerEngineImpl = &dockerMac{}
}
return containerEngineImpl, nil
Expand Down Expand Up @@ -555,7 +560,7 @@ func (o *consoleOptions) runConsoleContainer(ce containerEngineInterface) error

bridgeListen := fmt.Sprintf("http://0.0.0.0:%s", o.port)

envVars := []envVar{}
var envVars []envVar
// Set proxy URL to the container
proxyURL, err := getProxyURL()
if err != nil {
Expand Down Expand Up @@ -592,7 +597,7 @@ func (o *consoleOptions) runConsoleContainer(ce containerEngineInterface) error
"-listen", bridgeListen,
}

return ce.runConsoleContainer(consoleContainerName, o.port, containerArgs, envVars)
return ce.runConsoleContainer(o.ctx, o.cancel, consoleContainerName, o.port, containerArgs, envVars)
}

func (o *consoleOptions) runMonitorPlugin(ce containerEngineInterface) error {
Expand Down Expand Up @@ -683,7 +688,7 @@ func (o *consoleOptions) cleanUp(ce containerEngineInterface) error {
return err
}

containersToCleanUp := []string{}
var containersToCleanUp []string

// forcing order of removal as the order is not deterministic between container engines
if o.needMonitorPlugin {
Expand All @@ -698,7 +703,7 @@ func (o *consoleOptions) cleanUp(ce containerEngineInterface) error {
o.terminationFunction = &execActionOnTermStruct{}
}

<-ctx.Done()
<-o.ctx.Done()
err = o.terminationFunction.execActionOnTerminationFunction(func() error {
for _, c := range containersToCleanUp {
exist, err := ce.containerIsExist(c)
Expand Down Expand Up @@ -911,9 +916,6 @@ func GetConfigDirectory() (string, error) {

// Update config directory default path
pullSecretConfigDirectory = filepath.Join(home, ".kube/ocm-pull-secret")
if err != nil {
return "", fmt.Errorf("can't modify config directory. Error: %s", err.Error())
}
}

return pullSecretConfigDirectory, nil
Expand Down Expand Up @@ -1070,8 +1072,8 @@ func (ce *dockerMac) pullImage(imageName string) error {
return generalDockerPullImage(imageName)
}

// the shared function for podman to run console container for both linux and macos
func podmanRunConsoleContainer(containerName string, port string, consoleArgs []string, envVars []envVar) error {
// the shared function for podman to run console container for both linux and macOS
func podmanRunConsoleContainer(ctx context.Context, cancel context.CancelFunc, containerName string, port string, consoleArgs []string, envVars []envVar) error {
_, authFilename, err := fetchPullSecretIfNotExist()
if err != nil {
return err
Expand All @@ -1093,27 +1095,24 @@ func podmanRunConsoleContainer(containerName string, port string, consoleArgs []
engRunArgs = append(engRunArgs, consoleArgs...)
logger.WithField("Command", fmt.Sprintf("`%s %s`", PODMAN, strings.Join(engRunArgs, " "))).Infoln("Running container")

runCmd := exec.CommandContext(ctx, PODMAN, engRunArgs...)
runCmd := createCommandContext(ctx, PODMAN, engRunArgs...)
defer cancel()
runCmd.Stderr = os.Stderr
runCmd.Stdout = nil
err = runCmd.Run()
if err != nil {
return err
}
return nil

return runCmd.Run()
}

func (ce *podmanMac) runConsoleContainer(containerName string, port string, consoleArgs []string, envVars []envVar) error {
return podmanRunConsoleContainer(containerName, port, consoleArgs, envVars)
func (ce *podmanMac) runConsoleContainer(ctx context.Context, cancel context.CancelFunc, containerName string, port string, consoleArgs []string, envVars []envVar) error {
return podmanRunConsoleContainer(ctx, cancel, containerName, port, consoleArgs, envVars)
}

func (ce *podmanLinux) runConsoleContainer(containerName string, port string, consoleArgs []string, envVars []envVar) error {
return podmanRunConsoleContainer(containerName, port, consoleArgs, envVars)
func (ce *podmanLinux) runConsoleContainer(ctx context.Context, cancel context.CancelFunc, containerName string, port string, consoleArgs []string, envVars []envVar) error {
return podmanRunConsoleContainer(ctx, cancel, containerName, port, consoleArgs, envVars)
}

// the shared function for docker to run console container for both linux and macos
func dockerRunConsoleContainer(containerName string, port string, consoleArgs []string, envVars []envVar) error {
// the shared function for docker to run console container for both linux and macOS
func dockerRunConsoleContainer(ctx context.Context, cancel context.CancelFunc, containerName string, port string, consoleArgs []string, envVars []envVar) error {
configDirectory, _, err := fetchPullSecretIfNotExist()
if err != nil {
return err
Expand All @@ -1138,26 +1137,24 @@ func dockerRunConsoleContainer(containerName string, port string, consoleArgs []
}
engRunArgs = append(engRunArgs, consoleArgs...)
logger.WithField("Command", fmt.Sprintf("`%s %s`", DOCKER, strings.Join(engRunArgs, " "))).Infoln("Running container")
runCmd := createCommand(DOCKER, engRunArgs...)

runCmd := createCommandContext(ctx, DOCKER, engRunArgs...)
defer cancel()
runCmd.Stderr = os.Stderr
runCmd.Stdout = nil

err = runCmd.Run()
if err != nil {
return err
}
return nil
return runCmd.Run()
}

func (ce *dockerMac) runConsoleContainer(containerName string, port string, consoleArgs []string, envVars []envVar) error {
return dockerRunConsoleContainer(containerName, port, consoleArgs, envVars)
func (ce *dockerMac) runConsoleContainer(ctx context.Context, cancel context.CancelFunc, containerName string, port string, consoleArgs []string, envVars []envVar) error {
return dockerRunConsoleContainer(ctx, cancel, containerName, port, consoleArgs, envVars)
}

func (ce *dockerLinux) runConsoleContainer(containerName string, port string, consoleArgs []string, envVars []envVar) error {
return dockerRunConsoleContainer(containerName, port, consoleArgs, envVars)
func (ce *dockerLinux) runConsoleContainer(ctx context.Context, cancel context.CancelFunc, containerName string, port string, consoleArgs []string, envVars []envVar) error {
return dockerRunConsoleContainer(ctx, cancel, containerName, port, consoleArgs, envVars)
}

// the shared function for podman to run monitoring plugin for both linux and macos
// the shared function for podman to run monitoring plugin for both linux and macOS
func podmanRunMonitorPlugin(containerName string, consoleContainerName string, nginxConfPath string, pluginArgs []string) error {
_, authFilename, err := fetchPullSecretIfNotExist()
if err != nil {
Expand Down Expand Up @@ -1197,8 +1194,8 @@ func (ce *podmanLinux) runMonitorPlugin(containerName string, consoleContainerNa
return podmanRunMonitorPlugin(containerName, consoleContainerName, nginxConfPath, pluginArgs)
}

// the shared function for docker to run monitoring plugin for both linux and macos
func dockerRunMonitorPlugin(containerName string, consoleContainerName string, nginxConfPath string, pluginArgs []string) error {
// the shared function for docker to run monitoring plugin for both linux and macOS
func dockerRunMonitorPlugin(containerName string, _ string, nginxConfPath string, pluginArgs []string) error {
configDirectory, _, err := fetchPullSecretIfNotExist()
if err != nil {
return err
Expand Down Expand Up @@ -1385,7 +1382,7 @@ func (ce *podmanLinux) stopContainer(containerName string) error {
return generalStopContainer(PODMAN, containerName)
}

// podman-stop for MacOS
// podman-stop for macOS
func (ce *podmanMac) stopContainer(containerName string) error {
return generalStopContainer(PODMAN, containerName)
}
Expand All @@ -1395,7 +1392,7 @@ func (ce *dockerLinux) stopContainer(containerName string) error {
return generalStopContainer(DOCKER, containerName)
}

// docker-stop for MacOS
// docker-stop for macOS
func (ce *dockerMac) stopContainer(containerName string) error {
return generalStopContainer(DOCKER, containerName)
}
Expand All @@ -1405,7 +1402,7 @@ func (ce *podmanLinux) containerIsExist(containerName string) (bool, error) {
return generalContainerIsExist(PODMAN, containerName)
}

// podman-exist for MacOS
// podman-exist for macOS
func (ce *podmanMac) containerIsExist(containerName string) (bool, error) {
return generalContainerIsExist(PODMAN, containerName)
}
Expand All @@ -1415,7 +1412,7 @@ func (ce *dockerLinux) containerIsExist(containerName string) (bool, error) {
return generalContainerIsExist(DOCKER, containerName)
}

// docker-exist for MacOS
// docker-exist for macOS
func (ce *dockerMac) containerIsExist(containerName string) (bool, error) {
return generalContainerIsExist(DOCKER, containerName)
}
Loading

0 comments on commit 6731810

Please sign in to comment.