Skip to content

Commit

Permalink
Refactor
Browse files Browse the repository at this point in the history
  • Loading branch information
@anbrsap
anbrsap committed Nov 28, 2023
1 parent 31eedba commit 2b31da8
Show file tree
Hide file tree
Showing 13 changed files with 729 additions and 521 deletions.
1 change: 0 additions & 1 deletion go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ go 1.21
require (
github.com/benbjohnson/clock v1.3.5
github.com/davecgh/go-spew v1.1.1
github.com/ghodss/yaml v1.0.0
github.com/go-logr/logr v1.3.0
github.com/golang/mock v1.6.0
github.com/google/uuid v1.4.0
Expand Down
1 change: 0 additions & 1 deletion go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -685,7 +685,6 @@ github.com/evanphx/json-patch/v5 v5.7.0/go.mod h1:VNkHZ/282BpEyt/tObQO8s5CMPmYYq
github.com/flowstack/go-jsonschema v0.1.1/go.mod h1:yL7fNggx1o8rm9RlgXv7hTBWxdBM0rVwpMwimd3F3N0=
github.com/fogleman/gg v1.2.1-0.20190220221249-0403632d5b90/go.mod h1:R/bRT+9gY/C5z7JzPU0zXsXHKM4/ayA+zqcVNZzPa1k=
github.com/fogleman/gg v1.3.0/go.mod h1:R/bRT+9gY/C5z7JzPU0zXsXHKM4/ayA+zqcVNZzPa1k=
github.com/ghodss/yaml v1.0.0 h1:wQHKEahhL6wmXdzwWG11gIVCkOv05bNOh+Rxn0yngAk=
github.com/ghodss/yaml v1.0.0/go.mod h1:4dBDuWmgqj2HViK6kFavaiC9ZROes6MMH2rRYeMEF04=
github.com/go-fonts/dejavu v0.1.0/go.mod h1:4Wt4I4OU2Nq9asgDCteaAaWZOV24E+0/Pwo0gppep4g=
github.com/go-fonts/latin-modern v0.2.0/go.mod h1:rQVLdDMK+mK1xscDwsqM5J8U2jrRa3T0ecnM9pNujks=
Expand Down
11 changes: 2 additions & 9 deletions pkg/runctl/constants/constants.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,7 @@ import (
)

const (

// RunClusterRoleName is the name of the cluster role
// RunClusterRoleName is the name of the cluster role that
// pipeline run service accounts are bound to.
RunClusterRoleName k8s.RoleName = "steward-run"

// JFRStepName is the name of the jfs step
JFRStepName = "step-jenkinsfile-runner"

// TektonTaskRunName is the name of the Tekton TaskRun in each
// run namespace.
TektonTaskRunName = "steward-jenkinsfile-runner"
)
16 changes: 9 additions & 7 deletions pkg/runctl/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ func (c *Controller) newRunManager(workFactory k8s.ClientFactory, secretProvider
return c.testing.newRunManagerStub(workFactory, secretProvider)

}
return runmgr.NewRunManager(workFactory, secretProvider)
return runmgr.NewTektonRunManager(workFactory, secretProvider)
}

func (c *Controller) loadPipelineRunsConfig(ctx context.Context) (*cfg.PipelineRunsConfigStruct, error) {
Expand Down Expand Up @@ -445,7 +445,7 @@ func (c *Controller) handlePipelineRunFinalizerAndDeletion(
if pipelineRun.HasDeletionTimestamp() {
logger.V(3).Info("Unfinished pipeline run was deleted")
runManager := c.createRunManager(pipelineRun)
err := runManager.Cleanup(ctx, pipelineRun)
err := runManager.DeleteEnv(ctx, pipelineRun)
if err != nil {
c.eventRecorder.Event(pipelineRun.GetReference(), corev1.EventTypeWarning, api.EventReasonCleaningFailed, err.Error())
return true, err
Expand Down Expand Up @@ -532,7 +532,7 @@ func (c *Controller) handlePipelineRunPrepare(
"failed to load configuration for pipeline runs",
)
}
namespace, auxNamespace, err := runManager.Prepare(ctx, pipelineRun, pipelineRunsConfig)
namespace, auxNamespace, err := runManager.CreateEnv(ctx, pipelineRun, pipelineRunsConfig)
if err != nil {
c.eventRecorder.Event(pipelineRun.GetReference(), corev1.EventTypeWarning, api.EventReasonPreparingFailed, err.Error())
resultClass := serrors.GetClass(err)
Expand Down Expand Up @@ -641,11 +641,13 @@ func (c *Controller) handlePipelineRunWaiting(
return false, nil
}

func (c *Controller) startPipelineRun(ctx context.Context,
func (c *Controller) startPipelineRun(
ctx context.Context,
runManager run.Manager,
pipelineRun k8s.PipelineRun,
pipelineRunsConfig *cfg.PipelineRunsConfigStruct) error {
if err := runManager.Start(ctx, pipelineRun, pipelineRunsConfig); err != nil {
pipelineRunsConfig *cfg.PipelineRunsConfigStruct,
) error {
if err := runManager.CreateRun(ctx, pipelineRun, pipelineRunsConfig); err != nil {
c.eventRecorder.Event(pipelineRun.GetReference(), corev1.EventTypeWarning, api.EventReasonWaitingFailed, err.Error())
resultClass := serrors.GetClass(err)
// In case we have a result we can cleanup. Otherwise we retry in the next iteration.
Expand Down Expand Up @@ -722,7 +724,7 @@ func (c *Controller) handlePipelineRunCleaning(
if pipelineRun.GetStatus().State == api.StateCleaning {
logger.V(3).Info("Cleaning up pipeline execution")

err := runManager.Cleanup(ctx, pipelineRun)
err := runManager.DeleteEnv(ctx, pipelineRun)
if err != nil {
c.eventRecorder.Event(pipelineRun.GetReference(), corev1.EventTypeWarning, api.EventReasonCleaningFailed, err.Error())
return true, err
Expand Down
61 changes: 30 additions & 31 deletions pkg/runctl/controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
mocks "github.com/SAP/stewardci-core/pkg/k8s/mocks"
"github.com/SAP/stewardci-core/pkg/k8s/secrets"
cfg "github.com/SAP/stewardci-core/pkg/runctl/cfg"
"github.com/SAP/stewardci-core/pkg/runctl/constants"
metricstesting "github.com/SAP/stewardci-core/pkg/runctl/metrics/testing"
run "github.com/SAP/stewardci-core/pkg/runctl/run"
runmocks "github.com/SAP/stewardci-core/pkg/runctl/run/mocks"
Expand All @@ -36,7 +35,7 @@ import (
_ "knative.dev/pkg/system/testing"
)

func Test_Controller_meterAllPipelineRunsPeriodic(t *testing.T) {
func Test__Controller_meterAllPipelineRunsPeriodic(t *testing.T) {
// no parallel: patching global state

// SETUP
Expand Down Expand Up @@ -71,7 +70,7 @@ func Test_Controller_meterAllPipelineRunsPeriodic(t *testing.T) {
c.meterAllPipelineRunsPeriodic()
}

func Test_Controller_Success(t *testing.T) {
func Test__Controller__Success(t *testing.T) {
t.Parallel()

// SETUP
Expand All @@ -98,7 +97,7 @@ func Test_Controller_Success(t *testing.T) {
assert.Equal(t, 2, len(status.StateHistory))
}

func Test_Controller_Running(t *testing.T) {
func Test__Controller__Running(t *testing.T) {
t.Parallel()

for _, containerState := range []string{
Expand Down Expand Up @@ -146,14 +145,14 @@ func Test_Controller_Running(t *testing.T) {
func stepsWithContainer(state string, startTime metav1.Time) []tekton.StepState {
var stepState tekton.StepState
time, _ := json.Marshal(startTime)
s := fmt.Sprintf(`{ %q: {"startedAt": %s}, "container": %q, "name": "foo"}`, state, time, constants.JFRStepName)
s := fmt.Sprintf(`{ %q: {"startedAt": %s}, "name": %q}`, state, time, runmgr.JFRTaskRunStepName)
json.Unmarshal([]byte(s), &stepState)
return []tekton.StepState{
stepState,
}
}

func Test_Controller_Deletion(t *testing.T) {
func Test__Controller__Deletion(t *testing.T) {
t.Parallel()

// SETUP
Expand Down Expand Up @@ -184,7 +183,7 @@ func Test_Controller_Deletion(t *testing.T) {
assert.Equal(t, 0, len(run.GetFinalizers()))
}

func Test_Controller_syncHandler_PipelineRunNotFound(t *testing.T) {
func Test__Controller_syncHandler__PipelineRunNotFound(t *testing.T) {
t.Parallel()

// SETUP
Expand Down Expand Up @@ -241,7 +240,7 @@ func getAPIPipelineRun(cf *fake.ClientFactory, name, namespace string) (*api.Pip
return cs.StewardV1alpha1().PipelineRuns(namespace).Get(ctx, name, metav1.GetOptions{})
}

func Test_Controller_syncHandler_deleted_unfinished(t *testing.T) {
func Test__Controller_syncHandler__PipelineRunIsDeletedAndUnfinished(t *testing.T) {
for _, currentState := range []api.State{
api.StateUndefined,
api.StateNew,
Expand All @@ -263,7 +262,7 @@ func Test_Controller_syncHandler_deleted_unfinished(t *testing.T) {
name: "with_finalizer/cleanup_succeeds",
runManagerExpectation: func(rm *runmocks.MockManager) {
rm.EXPECT().
Cleanup(gomock.Any(), gomock.Any()).
DeleteEnv(gomock.Any(), gomock.Any()).
Return(nil)
},
withFinalizer: true,
Expand All @@ -276,7 +275,7 @@ func Test_Controller_syncHandler_deleted_unfinished(t *testing.T) {
name: "with_finalizer/cleanup_fails",
runManagerExpectation: func(rm *runmocks.MockManager) {
rm.EXPECT().
Cleanup(gomock.Any(), gomock.Any()).
DeleteEnv(gomock.Any(), gomock.Any()).
Return(errors.New("expected"))
},
withFinalizer: true,
Expand All @@ -289,7 +288,7 @@ func Test_Controller_syncHandler_deleted_unfinished(t *testing.T) {
name: "no_finalizer/cleanup_succeeds",
runManagerExpectation: func(rm *runmocks.MockManager) {
rm.EXPECT().
Cleanup(gomock.Any(), gomock.Any()).
DeleteEnv(gomock.Any(), gomock.Any()).
Return(nil)
},
withFinalizer: false,
Expand All @@ -302,7 +301,7 @@ func Test_Controller_syncHandler_deleted_unfinished(t *testing.T) {
name: "no_finalizer/cleanup_fails",
runManagerExpectation: func(rm *runmocks.MockManager) {
rm.EXPECT().
Cleanup(gomock.Any(), gomock.Any()).
DeleteEnv(gomock.Any(), gomock.Any()).
Return(errors.New("expected"))
},
withFinalizer: false,
Expand Down Expand Up @@ -360,7 +359,7 @@ func Test_Controller_syncHandler_deleted_unfinished(t *testing.T) {
}
}

func Test_Controller_syncHandler_deleted_finished(t *testing.T) {
func Test__Controller_syncHandler__PipelineRunIsDeletedAndFinished(t *testing.T) {
t.Parallel()

for _, currentResult := range []api.Result{
Expand Down Expand Up @@ -419,7 +418,7 @@ func Test_Controller_syncHandler_deleted_finished(t *testing.T) {
}
}

func Test_Controller_syncHandler_new(t *testing.T) {
func Test__Controller_syncHandler__PipelineRunIsNew(t *testing.T) {
error1 := errors.New("error1")
errorRecoverable1 := serrors.Recoverable(errors.New("errorRecoverable1"))

Expand All @@ -442,7 +441,7 @@ func Test_Controller_syncHandler_new(t *testing.T) {
pipelineRunSpec: api.PipelineSpec{},
runManagerExpectation: func(rm *runmocks.MockManager, run *runmocks.MockRun) {
rm.EXPECT().
Prepare(gomock.Any(), gomock.Any(), gomock.Any()).
CreateEnv(gomock.Any(), gomock.Any(), gomock.Any()).
Return("", "", nil)
},
expectedState: api.StateWaiting,
Expand All @@ -453,7 +452,7 @@ func Test_Controller_syncHandler_new(t *testing.T) {
pipelineRunSpec: api.PipelineSpec{},
runManagerExpectation: func(rm *runmocks.MockManager, run *runmocks.MockRun) {
rm.EXPECT().
Prepare(gomock.Any(), gomock.Any(), gomock.Any()).
CreateEnv(gomock.Any(), gomock.Any(), gomock.Any()).
Return("", "", error1)
},
expectedError: error1,
Expand Down Expand Up @@ -559,7 +558,7 @@ func Test_Controller_syncHandler_new(t *testing.T) {
}
}

func Test_Controller_syncHandler_unfinished(t *testing.T) {
func Test__Controller_syncHandler__PipelineRunIsUnfinished(t *testing.T) {
error1 := errors.New("error1")
errorRecoverable1 := serrors.Recoverable(errors.New("errorRecoverable1"))
longAgo := metav1.Unix(10, 10)
Expand Down Expand Up @@ -590,7 +589,7 @@ func Test_Controller_syncHandler_unfinished(t *testing.T) {
},
runManagerExpectation: func(rm *runmocks.MockManager, run *runmocks.MockRun) {
rm.EXPECT().
Prepare(gomock.Any(), gomock.Any(), gomock.Any()).
CreateEnv(gomock.Any(), gomock.Any(), gomock.Any()).
Return("", "", nil)
},
expectedState: api.StateWaiting,
Expand All @@ -605,7 +604,7 @@ func Test_Controller_syncHandler_unfinished(t *testing.T) {
},
runManagerExpectation: func(rm *runmocks.MockManager, run *runmocks.MockRun) {
rm.EXPECT().
Prepare(gomock.Any(), gomock.Any(), gomock.Any()).
CreateEnv(gomock.Any(), gomock.Any(), gomock.Any()).
Return("", "", error1)
},
expectedError: error1,
Expand All @@ -623,7 +622,7 @@ func Test_Controller_syncHandler_unfinished(t *testing.T) {
},
runManagerExpectation: func(rm *runmocks.MockManager, run *runmocks.MockRun) {
rm.EXPECT().
Prepare(gomock.Any(), gomock.Any(), gomock.Any()).
CreateEnv(gomock.Any(), gomock.Any(), gomock.Any()).
Return("", "", serrors.Classify(error1, api.ResultErrorContent))
},
expectedState: api.StateCleaning,
Expand All @@ -641,7 +640,7 @@ func Test_Controller_syncHandler_unfinished(t *testing.T) {
},
runManagerExpectation: func(rm *runmocks.MockManager, run *runmocks.MockRun) {
rm.EXPECT().
Prepare(gomock.Any(), gomock.Any(), gomock.Any()).
CreateEnv(gomock.Any(), gomock.Any(), gomock.Any()).
Return("", "", serrors.Classify(error1, api.ResultErrorContent))
},
expectedState: api.StateCleaning,
Expand All @@ -657,7 +656,7 @@ func Test_Controller_syncHandler_unfinished(t *testing.T) {
},
runManagerExpectation: func(rm *runmocks.MockManager, run *runmocks.MockRun) {
rm.EXPECT().
Prepare(gomock.Any(), gomock.Any(), gomock.Any()).
CreateEnv(gomock.Any(), gomock.Any(), gomock.Any()).
Return("", "", serrors.Classify(error1, api.ResultErrorInfra))
},
expectedState: api.StateCleaning,
Expand Down Expand Up @@ -789,7 +788,7 @@ func Test_Controller_syncHandler_unfinished(t *testing.T) {
GetRun(gomock.Any(), gomock.Any()).
Return(nil, nil)
rm.EXPECT().
Start(gomock.Any(), gomock.Any(), gomock.Any()).
CreateRun(gomock.Any(), gomock.Any(), gomock.Any()).
Return(nil)
},
expectedState: api.StateWaiting,
Expand All @@ -807,7 +806,7 @@ func Test_Controller_syncHandler_unfinished(t *testing.T) {
GetRun(gomock.Any(), gomock.Any()).
Return(nil, nil)
rm.EXPECT().
Start(gomock.Any(), gomock.Any(), gomock.Any()).
CreateRun(gomock.Any(), gomock.Any(), gomock.Any()).
Return(error1)
},
expectedError: error1,
Expand All @@ -826,7 +825,7 @@ func Test_Controller_syncHandler_unfinished(t *testing.T) {
GetRun(gomock.Any(), gomock.Any()).
Return(nil, nil)
rm.EXPECT().
Start(gomock.Any(), gomock.Any(), gomock.Any()).
CreateRun(gomock.Any(), gomock.Any(), gomock.Any()).
Return(serrors.Classify(error1, api.ResultErrorConfig))
},
expectedState: api.StateCleaning,
Expand Down Expand Up @@ -1219,7 +1218,7 @@ func Test_Controller_syncHandler_unfinished(t *testing.T) {
},
runManagerExpectation: func(rm *runmocks.MockManager, run *runmocks.MockRun) {
rm.EXPECT().
Cleanup(gomock.Any(), gomock.Any()).
DeleteEnv(gomock.Any(), gomock.Any()).
Return(nil)
},
expectedState: api.StateFinished,
Expand Down Expand Up @@ -1276,7 +1275,7 @@ func Test_Controller_syncHandler_unfinished(t *testing.T) {
},
runManagerExpectation: func(rm *runmocks.MockManager, run *runmocks.MockRun) {
rm.EXPECT().
Cleanup(gomock.Any(), gomock.Any()).
DeleteEnv(gomock.Any(), gomock.Any()).
Return(nil)
},
expectedState: api.StateFinished,
Expand Down Expand Up @@ -1345,7 +1344,7 @@ func Test_Controller_syncHandler_unfinished(t *testing.T) {
}
}

func Test_Controller_syncHandler_PipelineRunFetchFails_InternalServerError(t *testing.T) {
func Test__Controller_syncHandler__PipelineRunFetchFails_InternalServerError(t *testing.T) {
t.Parallel()

// SETUP
Expand Down Expand Up @@ -1376,7 +1375,7 @@ func Test_Controller_syncHandler_PipelineRunFetchFails_InternalServerError(t *te
assert.ErrorContains(t, err, message)
}

func Test_Controller_syncHandler_OnTimeout(t *testing.T) {
func Test__Controller_syncHandler__OnTimeout(t *testing.T) {
t.Parallel()

// SETUP
Expand Down Expand Up @@ -1459,7 +1458,7 @@ func Test_Controller_syncHandler_OnTimeout(t *testing.T) {
}

func newTestRunManager(workFactory k8s.ClientFactory, secretProvider secrets.SecretProvider) run.Manager {
return runmgr.NewRunManager(workFactory, secretProvider)
return runmgr.NewTektonRunManager(workFactory, secretProvider)
}

func startController(t *testing.T, cf *fake.ClientFactory) chan struct{} {
Expand Down Expand Up @@ -1545,7 +1544,7 @@ func updateRun(t *testing.T, run *api.PipelineRun, namespace string, cf *fake.Cl
func getTektonTaskRun(t *testing.T, namespace string, cf *fake.ClientFactory) *tekton.TaskRun {
t.Helper()
ctx := context.Background()
taskRun, err := cf.TektonV1beta1().TaskRuns(namespace).Get(ctx, constants.TektonTaskRunName, metav1.GetOptions{})
taskRun, err := cf.TektonV1beta1().TaskRuns(namespace).Get(ctx, runmgr.JFRTaskRunName, metav1.GetOptions{})
if err != nil {
t.Fatalf("could not get Tekton task run: %s", err.Error())
}
Expand Down
Loading

0 comments on commit 2b31da8

Please sign in to comment.