Skip to content

Commit

Permalink
Fix operation start to not start the same application twice.
Browse files Browse the repository at this point in the history
  • Loading branch information
blakerouse committed Jun 9, 2020
1 parent 3855965 commit 5b0c598
Show file tree
Hide file tree
Showing 9 changed files with 44 additions and 41 deletions.
2 changes: 1 addition & 1 deletion x-pack/elastic-agent/pkg/agent/operation/operation.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ type operation interface {
// examples:
// - Start does not need to run if process is running
// - Fetch does not need to run if package is already present
Check() (bool, error)
Check(application Application) (bool, error)
// Run runs the operation
Run(ctx context.Context, application Application) error
}
Expand Down
9 changes: 4 additions & 5 deletions x-pack/elastic-agent/pkg/agent/operation/operation_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,10 @@ func (o *operationConfig) Name() string {
return "operation-config"
}

// Check checks whether operation needs to be run
// examples:
// - Start does not need to run if process is running
// - Fetch does not need to run if package is already present
func (o *operationConfig) Check() (bool, error) { return true, nil }
// Check checks whether config needs to be run.
//
// Always returns true.
func (o *operationConfig) Check(_ Application) (bool, error) { return true, nil }

// Run runs the operation
func (o *operationConfig) Run(ctx context.Context, application Application) (err error) {
Expand Down
9 changes: 4 additions & 5 deletions x-pack/elastic-agent/pkg/agent/operation/operation_fetch.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,10 @@ func (o *operationFetch) Name() string {
return "operation-fetch"
}

// Check checks whether operation needs to be run
// examples:
// - Start does not need to run if process is running
// - Fetch does not need to run if package is already present
func (o *operationFetch) Check() (bool, error) {
// Check checks whether fetch needs to occur.
//
// If the artifacts already exists then fetch will not be ran.
func (o *operationFetch) Check(_ Application) (bool, error) {
downloadConfig := o.operatorConfig.DownloadConfig
fullPath, err := artifact.GetArtifactPath(o.program.BinaryName(), o.program.Version(), downloadConfig.OS(), downloadConfig.Arch(), downloadConfig.TargetDirectory)
if err != nil {
Expand Down
9 changes: 4 additions & 5 deletions x-pack/elastic-agent/pkg/agent/operation/operation_install.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,10 @@ func (o *operationInstall) Name() string {
return "operation-install"
}

// Check checks whether operation needs to be run
// examples:
// - Start does not need to run if process is running
// - Fetch does not need to run if package is already present
func (o *operationInstall) Check() (bool, error) {
// Check checks whether install needs to be ran.
//
// If the installation directory already exists then it will not be ran.
func (o *operationInstall) Check(_ Application) (bool, error) {
installDir := o.program.Directory()
_, err := os.Stat(installDir)
return os.IsNotExist(err), nil
Expand Down
9 changes: 4 additions & 5 deletions x-pack/elastic-agent/pkg/agent/operation/operation_remove.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,10 @@ func (o *operationRemove) Name() string {
return "operation-remove"
}

// Check checks whether operation needs to be run
// examples:
// - Start does not need to run if process is running
// - Fetch does not need to run if package is already present
func (o *operationRemove) Check() (bool, error) {
// Check checks whether remove needs to run.
//
// Always returns false.
func (o *operationRemove) Check(_ Application) (bool, error) {
return false, nil
}

Expand Down
20 changes: 12 additions & 8 deletions x-pack/elastic-agent/pkg/agent/operation/operation_start.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ package operation
import (
"context"

"github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/core/plugin/state"

"github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/agent/errors"
"github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/agent/operation/config"
"github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/core/logger"
Expand Down Expand Up @@ -47,14 +49,16 @@ func (o *operationStart) Name() string {
return "operation-start"
}

// Check checks whether operation needs to be run
// examples:
// - Start does not need to run if process is running
// - Fetch does not need to run if package is already present
func (o *operationStart) Check() (bool, error) {
// TODO: get running processes and compare hashes

return true, nil
// Check checks whether application needs to be started.
//
// Only starts the application when in stopped state, any other state
// and the application is handled by the life cycle inside of the `Application`
// implementation.
func (o *operationStart) Check(application Application) (bool, error) {
if application.State().Status == state.Stopped {
return true, nil
}
return false, nil
}

// Run runs the operation
Expand Down
16 changes: 10 additions & 6 deletions x-pack/elastic-agent/pkg/agent/operation/operation_stop.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ package operation
import (
"context"

"github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/core/plugin/state"

"github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/agent/errors"
"github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/agent/operation/config"
"github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/core/logger"
Expand Down Expand Up @@ -36,12 +38,14 @@ func (o *operationStop) Name() string {
return "operation-stop"
}

// Check checks whether operation needs to be run
// examples:
// - Start does not need to run if process is running
// - Fetch does not need to run if package is already present
func (o *operationStop) Check() (bool, error) {
return true, nil
// Check checks whether application needs to be stopped.
//
// If the application state is not stopped then stop should be performed.
func (o *operationStop) Check(application Application) (bool, error) {
if application.State().Status != state.Stopped {
return true, nil
}
return false, nil
}

// Run runs the operation
Expand Down
9 changes: 4 additions & 5 deletions x-pack/elastic-agent/pkg/agent/operation/operation_verify.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,10 @@ func (o *operationVerify) Name() string {
return "operation-verify"
}

// Check checks whether operation needs to be run
// examples:
// - Start does not need to run if process is running
// - Fetch does not need to run if package is already present
func (o *operationVerify) Check() (bool, error) {
// Check checks whether verify needs to occur.
//
// Only if the artifacts exists does it need to be verified.
func (o *operationVerify) Check(_ Application) (bool, error) {
downloadConfig := o.operatorConfig.DownloadConfig
fullPath, err := artifact.GetArtifactPath(o.program.BinaryName(), o.program.Version(), downloadConfig.OS(), downloadConfig.Arch(), downloadConfig.TargetDirectory)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion x-pack/elastic-agent/pkg/agent/operation/operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ func (o *Operator) runFlow(p Descriptor, operations []operation) error {
return err
}

shouldRun, err := op.Check()
shouldRun, err := op.Check(app)
if err != nil {
return err
}
Expand Down

0 comments on commit 5b0c598

Please sign in to comment.