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.

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

Update ExperimentManager interface and implement plugin methods for StandardExperimentManager #147

Merged
9 changes: 6 additions & 3 deletions api/turing/api/deployment_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"errors"
"strings"

"github.com/gojek/turing/engines/experiment/manager"

merlin "github.com/gojek/merlin/client"
mlp "github.com/gojek/mlp/api/client"
"github.com/gojek/turing/api/turing/models"
Expand Down Expand Up @@ -189,9 +191,10 @@ func (c RouterDeploymentController) deployRouterVersion(

expSvc := c.BaseController.AppContext.ExperimentsService
if routerVersion.ExperimentEngine.Type != models.ExperimentEngineTypeNop {
if expSvc.IsStandardExperimentManager(string(routerVersion.ExperimentEngine.Type)) {

if expSvc.IsStandardExperimentManager(routerVersion.ExperimentEngine.Type) {
// Convert the config to the standard type
expConfig, err := expSvc.GetStandardExperimentConfig(routerVersion.ExperimentEngine.Config)
expConfig, err := manager.ParseStandardExperimentConfig(routerVersion.ExperimentEngine.Config)
if err != nil {
return "", c.updateRouterVersionStatusToFailed(err, routerVersion)
}
Expand All @@ -206,7 +209,7 @@ func (c RouterDeploymentController) deployRouterVersion(

// Get the deployable Router Config for the experiment
experimentConfig, err = c.ExperimentsService.GetExperimentRunnerConfig(
string(routerVersion.ExperimentEngine.Type),
routerVersion.ExperimentEngine.Type,
routerVersion.ExperimentEngine.Config,
)
if err != nil {
Expand Down
22 changes: 9 additions & 13 deletions api/turing/api/deployment_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
"github.com/gojek/turing/api/turing/models"
"github.com/gojek/turing/api/turing/service/mocks"
"github.com/gojek/turing/api/turing/utils"
"github.com/gojek/turing/engines/experiment/manager"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
)
Expand All @@ -40,14 +39,12 @@ func TestDeployVersionSuccess(t *testing.T) {
nopExpCfg := &models.ExperimentEngine{
Type: "nop",
}
expCfg := &manager.TuringExperimentConfig{
Client: manager.Client{
ID: "1",
Passkey: "xp-passkey",
},
}
expCfg := json.RawMessage(`{"client": {"id": "1", "passkey": "test-passkey"}}`)

testEngineType := "test-manager"

expEnabledCfg := &models.ExperimentEngine{
Type: "xp",
Type: testEngineType,
Config: expCfg,
}

Expand Down Expand Up @@ -111,8 +108,8 @@ func TestDeployVersionSuccess(t *testing.T) {
ExperimentEngine: expEnabledCfg,
Status: "deployed",
},
expCfg: json.RawMessage([]byte(`{"engine": "xp"}`)),
decryptedPasskey: "xp-passkey-dec",
expCfg: json.RawMessage(`{"engine": "test"}`),
decryptedPasskey: "test-passkey-dec",
},
}

Expand All @@ -130,13 +127,12 @@ func TestDeployVersionSuccess(t *testing.T) {
es.On("Save", mock.Anything).Return(nil)

cs := &mocks.CryptoService{}
cs.On("Decrypt", "xp-passkey").Return("xp-passkey-dec", nil)
cs.On("Decrypt", "test-passkey").Return("test-passkey-dec", nil)

exps := &mocks.ExperimentsService{}
exps.On("IsStandardExperimentManager", "nop").Return(false)
exps.On("IsStandardExperimentManager", mock.Anything).Return(true)
exps.On("GetStandardExperimentConfig", expCfg).Return(*expCfg, nil)
exps.On("GetExperimentRunnerConfig", "xp", expCfg).Return(json.RawMessage([]byte(`{"engine": "xp"}`)), nil)
exps.On("GetExperimentRunnerConfig", testEngineType, expCfg).Return(json.RawMessage(`{"engine": "test"}`), nil)

// Run tests and validate
for name, data := range tests {
Expand Down
38 changes: 15 additions & 23 deletions api/turing/api/request/request.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package request

import (
"encoding/json"
"errors"
"fmt"

Expand Down Expand Up @@ -36,8 +37,8 @@ type RouterConfig struct {

// ExperimentEngineConfig defines the experiment engine config
type ExperimentEngineConfig struct {
Type string `json:"type" validate:"required,oneof=litmus nop xp"`
Config interface{} `json:"config,omitempty" validate:"-"` // Skip validate to invoke custom validation
Type string `json:"type" validate:"required"`
Config json.RawMessage `json:"config,omitempty" validate:"-"` // Skip validate to invoke custom validation
}

// LogConfig defines the logging configs
Expand Down Expand Up @@ -124,7 +125,7 @@ func (r CreateOrUpdateRouterRequest) BuildRouterVersion(
DefaultRouteID: r.Config.DefaultRouteID,
TrafficRules: r.Config.TrafficRules,
ExperimentEngine: &models.ExperimentEngine{
Type: models.ExperimentEngineType(r.Config.ExperimentEngine.Type),
Type: r.Config.ExperimentEngine.Type,
},
ResourceRequest: r.Config.ResourceRequest,
Timeout: r.Config.Timeout,
Expand All @@ -133,7 +134,7 @@ func (r CreateOrUpdateRouterRequest) BuildRouterVersion(
CustomMetricsEnabled: defaults.CustomMetricsEnabled,
FiberDebugLogEnabled: defaults.FiberDebugLogEnabled,
JaegerEnabled: defaults.JaegerEnabled,
ResultLoggerType: models.ResultLogger(r.Config.LogConfig.ResultLoggerType),
ResultLoggerType: r.Config.LogConfig.ResultLoggerType,
},
}
if r.Config.Enricher != nil {
Expand Down Expand Up @@ -179,54 +180,45 @@ func (r CreateOrUpdateRouterRequest) BuildExperimentEngineConfig(
router *models.Router,
cryptoSvc service.CryptoService,
expSvc service.ExperimentsService,
) (interface{}, error) {
) (json.RawMessage, error) {
rawExpConfig := r.Config.ExperimentEngine.Config

// Handle missing passkey / encrypt it in Standard experiment config
if expSvc.IsStandardExperimentManager(r.Config.ExperimentEngine.Type) {
// Convert the new config to the standard type
expConfig, err := expSvc.GetStandardExperimentConfig(rawExpConfig)
expConfig, err := manager.ParseStandardExperimentConfig(rawExpConfig)
if err != nil {
return nil, fmt.Errorf("Cannot parse standard experiment config: %v", err)
}
clientPasskey := expConfig.Client.Passkey

if clientPasskey == "" {
if expConfig.Client.Passkey == "" {
// Extract existing router version config
if router.CurrRouterVersion != nil &&
string(router.CurrRouterVersion.ExperimentEngine.Type) == r.Config.ExperimentEngine.Type {
currVerExpConfig, err := expSvc.GetStandardExperimentConfig(router.CurrRouterVersion.ExperimentEngine.Config)
router.CurrRouterVersion.ExperimentEngine.Type == r.Config.ExperimentEngine.Type {
currVerExpConfig, err := manager.ParseStandardExperimentConfig(router.CurrRouterVersion.ExperimentEngine.Config)
if err != nil {
return nil, fmt.Errorf("Error parsing existing experiment config: %v", err)
}
if expConfig.Client.Username == currVerExpConfig.Client.Username {
// Copy the passkey
clientPasskey = currVerExpConfig.Client.Passkey
expConfig.Client.Passkey = currVerExpConfig.Client.Passkey
}
}
// If the passkey is still empty, we cannot proceed
if clientPasskey == "" {
if expConfig.Client.Passkey == "" {
return nil, errors.New("Passkey must be configured")
}
} else {
// Passkey has been supplied, encrypt it
var err error
clientPasskey, err = cryptoSvc.Encrypt(clientPasskey)
expConfig.Client.Passkey, err = cryptoSvc.Encrypt(expConfig.Client.Passkey)
if err != nil {
return nil, fmt.Errorf("Passkey could not be encrypted: %s", err.Error())
}
}

// Build Experiment engine config
return &manager.TuringExperimentConfig{
Client: manager.Client{
ID: expConfig.Client.ID,
Username: expConfig.Client.Username,
Passkey: clientPasskey,
},
Experiments: expConfig.Experiments,
Variables: expConfig.Variables,
}, nil
// Marshal Experiment engine config
return json.Marshal(expConfig)
}

// Custom experiment manager config, return as is.
Expand Down
Loading