Skip to content

Introduce ForceRerun parameter for Linux RCv2 #65

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

Closed
viveklingaiah wants to merge 1 commit into from
Closed

Introduce ForceRerun parameter for Linux RCv2 #65

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
17 changes: 13 additions & 4 deletions internal/cmds/cmds.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,9 +165,18 @@ func uninstall(ctx *log.Context, h types.HandlerEnvironment, report *types.RunCo
}

func enablePre(ctx *log.Context, h types.HandlerEnvironment, metadata types.RCMetadata, c types.Cmd) error {
extensionEvents := createExtensionEventManager(ctx, h)

// parse the extension handler settings
cfg, err1 := handlersettings.GetHandlerSettings(h.HandlerEnvironment.ConfigFolder, metadata.ExtName, metadata.SeqNum, ctx)
if err1 != nil {
errMessage := fmt.Sprintf("Failed to get configuration in enablePre: %v", err1)
extensionEvents.LogErrorEvent("enablePre", errMessage)
return errors.Wrap(err1, "failed to get configuration")
}
// exit if this sequence number (a snapshot of the configuration) is already
// processed. if not, save this sequence number before proceeding.
if shouldExit, err := checkAndSaveSeqNum(ctx, metadata.SeqNum, metadata.MostRecentSequence); err != nil {
if shouldExit, err := checkAndSaveSeqNum(ctx, metadata.SeqNum, metadata.MostRecentSequence, cfg.ForceRerun); err != nil {
return errors.Wrap(err, "failed to process sequence number")
} else if shouldExit {
ctx.Log("event", "exit", "message", "the script configuration has already been processed, will not run again")
Expand Down Expand Up @@ -388,14 +397,14 @@ func getOutput(ctx *log.Context, stdoutFileName string, stderrFileName string) (
// checkAndSaveSeqNum checks if the given seqNum is already processed
// according to the specified seqNumFile and if so, returns true,
// otherwise saves the given seqNum into seqNumFile returns false.
func checkAndSaveSeqNum(ctx log.Logger, seq int, mrseqPath string) (shouldExit bool, _ error) {
func checkAndSaveSeqNum(ctx log.Logger, seq int, mrseqPath string, forceRerun bool) (shouldExit bool, _ error) {
ctx.Log("event", "comparing seqnum", "path", mrseqPath)
smaller, err := seqnum.IsSmallerThan(mrseqPath, seq)
isSavedSeqNumSmallerThanNewSeqNum, err := seqnum.IsSmallerThan(mrseqPath, seq)

@jscalev jscalev Sep 2, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should trace somewhere (and also put it in an extension event) that the force flag was used.

if err != nil {
return false, errors.Wrap(err, "failed to check sequence number")
}

if !smaller {
if !forceRerun && !isSavedSeqNumSmallerThanNewSeqNum {

@jscalev jscalev Sep 2, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What happens if the machine is rebooted? From this it appears we'll run the script every time RCV2 is executed. Why not just have CRP increment the seqNo?

@viveklingaiah viveklingaiah Sep 3, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good idea. I did think about it earlier. Do you mean instead of making this "ForceRun" (thinking to rename it) flag change in Windows and Linux extensions, we could just increment sequence number in CRP ? Only thing is it may not work right away if for some reason the VM has much higher sequence number than what CRP has. Especially, Linux uses files to note down mrseq number and if user reuses a disk for example. That may be an edge case though.

@viveklingaiah viveklingaiah Sep 3, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think just incrementing it in CRP is a better solution. I'll close this PR

// stored sequence number is equals or greater than the current
// sequence number.
return true, nil
Expand Down
37 changes: 31 additions & 6 deletions internal/cmds/cmds_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ func Test_commands_shouldReportStatus(t *testing.T) {

func Test_checkAndSaveSeqNum_fails(t *testing.T) {
// pass in invalid seqnum format
_, err := checkAndSaveSeqNum(log.NewNopLogger(), 0, "/non/existing/dir")
_, err := checkAndSaveSeqNum(log.NewNopLogger(), 0, "/non/existing/dir", false)
require.NotNil(t, err)
require.Contains(t, err.Error(), `failed to save sequence number`)
}
Expand All @@ -162,27 +162,52 @@ func Test_checkAndSaveSeqNum(t *testing.T) {
nop := log.NewNopLogger()

// no sequence number, 0 comes in.
shouldExit, err := checkAndSaveSeqNum(nop, 0, fp)
shouldExit, err := checkAndSaveSeqNum(nop, 0, fp, false)
require.Nil(t, err)
require.False(t, shouldExit)

// file=0, seq=0 comes in. (should exit)
shouldExit, err = checkAndSaveSeqNum(nop, 0, fp)
shouldExit, err = checkAndSaveSeqNum(nop, 0, fp, false)
require.Nil(t, err)
require.True(t, shouldExit)

// file=0, seq=1 comes in.
shouldExit, err = checkAndSaveSeqNum(nop, 1, fp)
shouldExit, err = checkAndSaveSeqNum(nop, 1, fp, false)
require.Nil(t, err)
require.False(t, shouldExit)

// file=1, seq=1 comes in. (should exit)
shouldExit, err = checkAndSaveSeqNum(nop, 1, fp)
shouldExit, err = checkAndSaveSeqNum(nop, 1, fp, false)
require.Nil(t, err)
require.True(t, shouldExit)

// file=1, seq=0 comes in. (should exit)
shouldExit, err = checkAndSaveSeqNum(nop, 1, fp)
shouldExit, err = checkAndSaveSeqNum(nop, 0, fp, false)
require.Nil(t, err)
require.True(t, shouldExit)

// file=1, seq=0 comes in. ForceRerun=true, should not exit
shouldExit, err = checkAndSaveSeqNum(nop, 0, fp, true)
require.Nil(t, err)
require.False(t, shouldExit)

// file=0, seq=0 comes in. ForceRerun=true, should not exit
shouldExit, err = checkAndSaveSeqNum(nop, 0, fp, true)
require.Nil(t, err)
require.False(t, shouldExit)

// file=0, seq=1 comes in. ForceRerun=true, should not exit
shouldExit, err = checkAndSaveSeqNum(nop, 1, fp, true)
require.Nil(t, err)
require.False(t, shouldExit)

// file=1, seq=5 comes in. ForceRerun=true, should not exit
shouldExit, err = checkAndSaveSeqNum(nop, 1, fp, true)
require.Nil(t, err)
require.False(t, shouldExit)

// file=5, seq=0 comes in. ForceRerun=false, should exit
shouldExit, err = checkAndSaveSeqNum(nop, 0, fp, false)
require.Nil(t, err)
require.True(t, shouldExit)
}
Expand Down
1 change: 1 addition & 0 deletions internal/handlersettings/types.go
100644 → 100755
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ type PublicSettings struct {
TimeoutInSeconds int `json:"timeoutInSeconds,int"`
AsyncExecution bool `json:"asyncExecution,bool"`
TreatFailureAsDeploymentFailure bool `json:"treatFailureAsDeploymentFailure,bool"`
ForceRerun bool `json:"forceRerun,bool"`

// List of artifacts to download before running the script
Artifacts []PublicArtifactSource `json:"artifacts"`
Expand Down
Loading