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

Improve signal handling - Make the state machine context aware #231

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+486 −459
Open

Improve signal handling - Make the state machine context aware #231

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0eb091b
Rename states to stateFuncs
upils May 27, 2024
16b33d9
Catch SIGINT and SIGTERM but simply exit for now
upils May 27, 2024
e58f999
Support context and cancelFuncs in stateMachine
upils Jul 5, 2024
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
15 changes: 14 additions & 1 deletion cmd/ubuntu-image/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
package main

import (
"context"
"fmt"
"io"
"os"
"os/signal"
"syscall"

"github.com/jessevdk/go-flags"

Expand Down Expand Up @@ -55,7 +58,7 @@ func executeStateMachine(sm statemachine.SmInterface) error {
return err
}

if err := sm.Run(); err != nil {
if err := sm.Run(context.Background()); err != nil {
return err
}

Expand Down Expand Up @@ -191,6 +194,16 @@ func main() { //nolint: gocyclo
imageType = parser.Command.Active.Name
}

// Properly handle signals to execute defered functions and make sure
// mounted dir are unmounted
ch := make(chan os.Signal, 2)
signal.Notify(ch, syscall.SIGINT, syscall.SIGTERM)

go func() {
Copy link

Choose a reason for hiding this comment

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

Do you already have plans for the signal handler? Like, if the cleanup process blocks or takes too long for some reason (like trying to call umount on a problematic mounting point (I don't know, imagine a bad disk or NFS mount that got stuck)) it should be possible to still interrupt the tool (you could kill -9 it anyway).

If you ctrl+c again while the signal handler is running nothing will happen because the signal is being caught. You could handle subsequent ctrl+c's and inform the user that the tool is shutting down but if they want to stop it anyway they can hit ctrl+c again.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes! That is the plan in the following PR #235 (still very much WIP)

<-ch
osExit(1)
}()

// init the state machine
sm, err := initStateMachine(imageType, commonOpts, stateMachineOpts, ubuntuImageCommand)
if err != nil {
Expand Down
3 changes: 2 additions & 1 deletion cmd/ubuntu-image/main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package main

import (
"context"
"errors"
"flag"
"io"
Expand Down Expand Up @@ -35,7 +36,7 @@ func (mockSM *mockedStateMachine) Setup() error {
return nil
}

func (mockSM *mockedStateMachine) Run() error {
func (mockSM *mockedStateMachine) Run(ctx context.Context) error {
if mockSM.whenToFail == "Run" {
return ErrAtRun
}
Expand Down
4 changes: 2 additions & 2 deletions internal/statemachine/classic.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func (classicStateMachine *ClassicStateMachine) Setup() error {
// set the parent pointer of the embedded struct
classicStateMachine.parent = classicStateMachine

classicStateMachine.states = make([]stateFunc, 0)
classicStateMachine.stateFuncs = make([]stateFunc, 0)

if err := classicStateMachine.setConfDefDir(classicStateMachine.parent.(*ClassicStateMachine).Args.ImageDefinition); err != nil {
return err
Expand Down Expand Up @@ -379,7 +379,7 @@ func (s *StateMachine) calculateStates() error {
s.addArtifactsStates(c, &rootfsCreationStates)

// Append the newly calculated states to the slice of funcs in the parent struct
s.states = append(s.states, rootfsCreationStates...)
s.stateFuncs = append(s.stateFuncs, rootfsCreationStates...)

return nil
}
Expand Down
144 changes: 72 additions & 72 deletions internal/statemachine/classic_states.go
View file
Open in desktop

Large diffs are not rendered by default.

Loading
Loading