Skip to content

Commit

Permalink
Rename Ownership labels (#126)
Browse files Browse the repository at this point in the history
  • Loading branch information
jmendesky authored Jun 27, 2022
1 parent 9a15819 commit 89462db
Show file tree
Hide file tree
Showing 29 changed files with 257 additions and 262 deletions.
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,6 @@ dist/
node_modules/
package-lock.json
.hugo_build.lock

# OS
.DS_Store
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ integration-test-up:
integration-test: manifests generate ## Run integration tests
eval $$(minikube -p kfp-operator-tests docker-env) && \
$(MAKE) docker-build-argo && \
docker build docs/quickstart -t kfp-quickstart
docker build docs-gen/includes/quickstart -t kfp-quickstart
go test ./... -tags=integration

integration-test-down:
Expand Down
2 changes: 1 addition & 1 deletion apis/pipelines/v1/experiment_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func (e *Experiment) SetStatus(status Status) {
e.Status = status
}

func (e Experiment) NamespacedName() types.NamespacedName {
func (e Experiment) GetNamespacedName() types.NamespacedName {
return types.NamespacedName{
Name: e.Name,
Namespace: e.Namespace,
Expand Down
2 changes: 1 addition & 1 deletion apis/pipelines/v1/pipeline_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func (p *Pipeline) SetStatus(status Status) {
p.Status = status
}

func (p Pipeline) NamespacedName() types.NamespacedName {
func (p Pipeline) GetNamespacedName() types.NamespacedName {
return types.NamespacedName{
Name: p.Name,
Namespace: p.Namespace,
Expand Down
2 changes: 1 addition & 1 deletion apis/pipelines/v1/runconfiguration_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func (rc *RunConfiguration) SetStatus(status Status) {
rc.Status.Status = status
}

func (rc RunConfiguration) NamespacedName() types.NamespacedName {
func (rc RunConfiguration) GetNamespacedName() types.NamespacedName {
return types.NamespacedName{
Name: rc.Name,
Namespace: rc.Namespace,
Expand Down
12 changes: 6 additions & 6 deletions controllers/pipelines/experiment_controller_decoupled_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ var _ = Describe("Experiment controller k8s integration", Serial, func() {
g.Expect(experiment.Status.ObservedGeneration).To(Equal(experiment.GetGeneration()))
})).Should(Succeed())

Eventually(testCtx.WorkflowToBeUpdated(ExperimentWorkflowConstants.CreateOperationLabel, func(workflow *argo.Workflow) {
Eventually(testCtx.WorkflowToBeUpdated(WorkflowConstants.CreateOperationLabel, func(workflow *argo.Workflow) {
workflow.Status.Phase = argo.WorkflowSucceeded
setWorkflowOutputs(
workflow,
Expand All @@ -44,7 +44,7 @@ var _ = Describe("Experiment controller k8s integration", Serial, func() {
Eventually(testCtx.ExperimentToMatch(func(g Gomega, experiment *pipelinesv1.Experiment) {
g.Expect(experiment.Status.SynchronizationState).To(Equal(pipelinesv1.Succeeded))
})).Should(Succeed())
Eventually(testCtx.FetchWorkflow(ExperimentWorkflowConstants.CreateOperationLabel)).Should(Not(Succeed()))
Eventually(testCtx.FetchWorkflow(WorkflowConstants.CreateOperationLabel)).Should(Not(Succeed()))

Expect(testCtx.UpdateExperiment(func(pipeline *pipelinesv1.Experiment) {
pipeline.Spec = RandomExperimentSpec()
Expand All @@ -54,7 +54,7 @@ var _ = Describe("Experiment controller k8s integration", Serial, func() {
g.Expect(experiment.Status.SynchronizationState).To(Equal(pipelinesv1.Updating))
})).Should(Succeed())

Eventually(testCtx.WorkflowToBeUpdated(ExperimentWorkflowConstants.UpdateOperationLabel, func(workflow *argo.Workflow) {
Eventually(testCtx.WorkflowToBeUpdated(WorkflowConstants.UpdateOperationLabel, func(workflow *argo.Workflow) {
workflow.Status.Phase = argo.WorkflowSucceeded
setWorkflowOutputs(
workflow,
Expand All @@ -70,20 +70,20 @@ var _ = Describe("Experiment controller k8s integration", Serial, func() {
Eventually(testCtx.ExperimentToMatch(func(g Gomega, experiment *pipelinesv1.Experiment) {
g.Expect(experiment.Status.SynchronizationState).To(Equal(pipelinesv1.Succeeded))
})).Should(Succeed())
Eventually(testCtx.FetchWorkflow(ExperimentWorkflowConstants.UpdateOperationLabel)).Should(Not(Succeed()))
Eventually(testCtx.FetchWorkflow(WorkflowConstants.UpdateOperationLabel)).Should(Not(Succeed()))

Expect(testCtx.DeleteExperiment()).To(Succeed())

Eventually(testCtx.ExperimentToMatch(func(g Gomega, experiment *pipelinesv1.Experiment) {
g.Expect(experiment.Status.SynchronizationState).To(Equal(pipelinesv1.Deleting))
})).Should(Succeed())

Eventually(testCtx.WorkflowToBeUpdated(ExperimentWorkflowConstants.DeleteOperationLabel, func(workflow *argo.Workflow) {
Eventually(testCtx.WorkflowToBeUpdated(WorkflowConstants.DeleteOperationLabel, func(workflow *argo.Workflow) {
workflow.Status.Phase = argo.WorkflowSucceeded
})).Should(Succeed())

Eventually(testCtx.ExperimentExists).Should(Not(Succeed()))
Eventually(testCtx.FetchWorkflow(ExperimentWorkflowConstants.DeleteOperationLabel)).Should(Not(Succeed()))
Eventually(testCtx.FetchWorkflow(WorkflowConstants.DeleteOperationLabel)).Should(Not(Succeed()))

Eventually(testCtx.EmittedEventsToMatch(func(g Gomega, events []v1.Event) {
g.Expect(events).To(ConsistOf(
Expand Down
16 changes: 8 additions & 8 deletions controllers/pipelines/experiment_state_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ func (st *ExperimentStateHandler) stateTransition(ctx context.Context, experimen
case pipelinesv1.Creating:
commands = st.onCreating(ctx, experiment,
st.WorkflowRepository.GetByLabels(ctx, experiment.GetNamespace(),
st.WorkflowFactory.Labels(experiment, ExperimentWorkflowConstants.CreateOperationLabel)))
CommonWorkflowLabels(experiment, WorkflowConstants.CreateOperationLabel)))
case pipelinesv1.Succeeded, pipelinesv1.Failed:
if !experiment.ObjectMeta.DeletionTimestamp.IsZero() {
commands = st.onDelete(ctx, experiment)
Expand All @@ -28,11 +28,11 @@ func (st *ExperimentStateHandler) stateTransition(ctx context.Context, experimen
case pipelinesv1.Updating:
commands = st.onUpdating(ctx, experiment,
st.WorkflowRepository.GetByLabels(ctx, experiment.GetNamespace(),
st.WorkflowFactory.Labels(experiment, ExperimentWorkflowConstants.UpdateOperationLabel)))
CommonWorkflowLabels(experiment, WorkflowConstants.UpdateOperationLabel)))
case pipelinesv1.Deleting:
commands = st.onDeleting(ctx, experiment,
st.WorkflowRepository.GetByLabels(ctx, experiment.GetNamespace(),
st.WorkflowFactory.Labels(experiment, ExperimentWorkflowConstants.DeleteOperationLabel)))
CommonWorkflowLabels(experiment, WorkflowConstants.DeleteOperationLabel)))
case pipelinesv1.Deleted:
default:
commands = st.onUnknown(ctx, experiment)
Expand Down Expand Up @@ -62,7 +62,7 @@ func (st *ExperimentStateHandler) onUnknown(ctx context.Context, experiment *pip

if experiment.Status.KfpId != "" {
logger.Info("empty state but KfpId already exists, updating experiment")
workflow, err := st.WorkflowFactory.ConstructUpdateWorkflow(ctx, experiment)
workflow, err := st.WorkflowFactory.ConstructUpdateWorkflow(experiment)

if err != nil {
failureMessage := "error constructing update workflow"
Expand All @@ -82,7 +82,7 @@ func (st *ExperimentStateHandler) onUnknown(ctx context.Context, experiment *pip
}

logger.Info("empty state, creating experiment")
workflow, err := st.WorkflowFactory.ConstructCreationWorkflow(ctx, experiment)
workflow, err := st.WorkflowFactory.ConstructCreationWorkflow(experiment)

if err != nil {
failureMessage := "error constructing creation workflow"
Expand Down Expand Up @@ -114,7 +114,7 @@ func (st ExperimentStateHandler) onDelete(ctx context.Context, experiment *pipel
}
}

workflow, err := st.WorkflowFactory.ConstructDeletionWorkflow(ctx, experiment)
workflow, err := st.WorkflowFactory.ConstructDeletionWorkflow(experiment)

if err != nil {
failureMessage := "error constructing deletion workflow"
Expand Down Expand Up @@ -146,7 +146,7 @@ func (st ExperimentStateHandler) onSucceededOrFailed(ctx context.Context, experi

if experiment.Status.KfpId == "" {
logger.Info("no kfpId exists, creating")
workflow, err = st.WorkflowFactory.ConstructCreationWorkflow(ctx, experiment)
workflow, err = st.WorkflowFactory.ConstructCreationWorkflow(experiment)

if err != nil {
failureMessage := "error constructing creation workflow"
Expand All @@ -160,7 +160,7 @@ func (st ExperimentStateHandler) onSucceededOrFailed(ctx context.Context, experi
targetState = pipelinesv1.Creating
} else {
logger.Info("kfpId exists, updating")
workflow, err = st.WorkflowFactory.ConstructUpdateWorkflow(ctx, experiment)
workflow, err = st.WorkflowFactory.ConstructUpdateWorkflow(experiment)

if err != nil {
failureMessage := "error constructing update workflow"
Expand Down
16 changes: 8 additions & 8 deletions controllers/pipelines/experiment_state_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,13 @@ func (st ExperimentStateTransitionTestCase) WithWorkFlow(workflow *argo.Workflow
}

func (st ExperimentStateTransitionTestCase) WithCreateWorkFlow(phase argo.WorkflowPhase) ExperimentStateTransitionTestCase {
return st.WithWorkFlow(CreateTestWorkflow(ExperimentWorkflowConstants.CreateOperationLabel, phase))
return st.WithWorkFlow(CreateTestWorkflow(WorkflowConstants.CreateOperationLabel, phase))
}

func (st ExperimentStateTransitionTestCase) WithCreateWorkFlowWithId(phase argo.WorkflowPhase, kfpId string) ExperimentStateTransitionTestCase {
return st.WithWorkFlow(
setWorkflowOutputs(
CreateTestWorkflow(ExperimentWorkflowConstants.CreateOperationLabel, phase),
CreateTestWorkflow(WorkflowConstants.CreateOperationLabel, phase),
[]argo.Parameter{
{
Name: ExperimentWorkflowConstants.ExperimentIdParameterName,
Expand All @@ -47,14 +47,14 @@ func (st ExperimentStateTransitionTestCase) WithCreateWorkFlowWithId(phase argo.

func (st ExperimentStateTransitionTestCase) WithFailedUpdateWorkflow() ExperimentStateTransitionTestCase {
return st.WithWorkFlow(
CreateTestWorkflow(ExperimentWorkflowConstants.UpdateOperationLabel, argo.WorkflowFailed),
CreateTestWorkflow(WorkflowConstants.UpdateOperationLabel, argo.WorkflowFailed),
)
}

func (st ExperimentStateTransitionTestCase) WithSucceededUpdateWorkflowWithId(kfpId string) ExperimentStateTransitionTestCase {
return st.WithWorkFlow(
setWorkflowOutputs(
CreateTestWorkflow(ExperimentWorkflowConstants.UpdateOperationLabel, argo.WorkflowSucceeded),
CreateTestWorkflow(WorkflowConstants.UpdateOperationLabel, argo.WorkflowSucceeded),
[]argo.Parameter{
{
Name: ExperimentWorkflowConstants.ExperimentIdParameterName,
Expand All @@ -67,22 +67,22 @@ func (st ExperimentStateTransitionTestCase) WithSucceededUpdateWorkflowWithId(kf

func (st ExperimentStateTransitionTestCase) WithDeletionWorkflow(phase argo.WorkflowPhase) ExperimentStateTransitionTestCase {
return st.WithWorkFlow(
CreateTestWorkflow(ExperimentWorkflowConstants.DeleteOperationLabel, phase),
CreateTestWorkflow(WorkflowConstants.DeleteOperationLabel, phase),
)
}

func (st ExperimentStateTransitionTestCase) IssuesCreationWorkflow() ExperimentStateTransitionTestCase {
creationWorkflow, _ := st.workflowFactory.ConstructCreationWorkflow(context.Background(), st.Experiment)
creationWorkflow, _ := st.workflowFactory.ConstructCreationWorkflow(st.Experiment)
return st.IssuesCommand(CreateWorkflow{Workflow: *creationWorkflow})
}

func (st ExperimentStateTransitionTestCase) IssuesUpdateWorkflow() ExperimentStateTransitionTestCase {
updateWorkflow, _ := st.workflowFactory.ConstructUpdateWorkflow(context.Background(), st.Experiment)
updateWorkflow, _ := st.workflowFactory.ConstructUpdateWorkflow(st.Experiment)
return st.IssuesCommand(CreateWorkflow{Workflow: *updateWorkflow})
}

func (st ExperimentStateTransitionTestCase) IssuesDeletionWorkflow() ExperimentStateTransitionTestCase {
deletionWorkflow, _ := st.workflowFactory.ConstructDeletionWorkflow(context.Background(), st.Experiment)
deletionWorkflow, _ := st.workflowFactory.ConstructDeletionWorkflow(st.Experiment)
return st.IssuesCommand(CreateWorkflow{Workflow: *deletionWorkflow})
}

Expand Down
18 changes: 8 additions & 10 deletions controllers/pipelines/experiment_test_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,9 @@ type ExperimentTestContext struct {
func NewExperimentTestContext(experiment *pipelinesv1.Experiment, k8sClient client.Client, ctx context.Context) ExperimentTestContext {
return ExperimentTestContext{
TestContext: TestContext{
K8sClient: k8sClient,
ctx: ctx,
LookupKey: experiment.NamespacedName(),
LookupLabel: ExperimentWorkflowConstants.ExperimentNameLabelKey,
operationLabel: ExperimentWorkflowConstants.OperationLabelKey,
K8sClient: k8sClient,
ctx: ctx,
Resource: experiment,
},
Experiment: experiment,
}
Expand All @@ -32,22 +30,22 @@ func NewExperimentTestContext(experiment *pipelinesv1.Experiment, k8sClient clie
func (testCtx ExperimentTestContext) ExperimentToMatch(matcher func(Gomega, *pipelinesv1.Experiment)) func(Gomega) {
return func(g Gomega) {
rc := &pipelinesv1.Experiment{}
Expect(testCtx.K8sClient.Get(testCtx.ctx, testCtx.LookupKey, rc)).To(Succeed())
Expect(testCtx.K8sClient.Get(testCtx.ctx, testCtx.Resource.GetNamespacedName(), rc)).To(Succeed())
matcher(g, rc)
}
}

func (testCtx ExperimentTestContext) ExperimentExists() error {
rc := &pipelinesv1.Experiment{}
err := testCtx.K8sClient.Get(testCtx.ctx, testCtx.LookupKey, rc)
err := testCtx.K8sClient.Get(testCtx.ctx, testCtx.Resource.GetNamespacedName(), rc)

return err
}

func (testCtx ExperimentTestContext) UpdateExperiment(updateFunc func(*pipelinesv1.Experiment)) error {
rc := &pipelinesv1.Experiment{}

if err := testCtx.K8sClient.Get(testCtx.ctx, testCtx.LookupKey, rc); err != nil {
if err := testCtx.K8sClient.Get(testCtx.ctx, testCtx.Resource.GetNamespacedName(), rc); err != nil {
return err
}

Expand All @@ -59,7 +57,7 @@ func (testCtx ExperimentTestContext) UpdateExperiment(updateFunc func(*pipelines
func (testCtx ExperimentTestContext) DeleteExperiment() error {
rc := &pipelinesv1.Experiment{}

if err := testCtx.K8sClient.Get(testCtx.ctx, testCtx.LookupKey, rc); err != nil {
if err := testCtx.K8sClient.Get(testCtx.ctx, testCtx.Resource.GetNamespacedName(), rc); err != nil {
return err
}

Expand All @@ -69,7 +67,7 @@ func (testCtx ExperimentTestContext) DeleteExperiment() error {
func (testCtx ExperimentTestContext) EmittedEventsToMatch(matcher func(Gomega, []v1.Event)) func(Gomega) {
return func(g Gomega) {
eventList := &v1.EventList{}
Expect(testCtx.K8sClient.List(testCtx.ctx, eventList, client.MatchingFields{"involvedObject.name": testCtx.Experiment.Name})).To(Succeed())
Expect(testCtx.K8sClient.List(testCtx.ctx, eventList, client.MatchingFields{"involvedObject.name": testCtx.Resource.GetName()})).To(Succeed())

matcher(g, eventList.Items)
}
Expand Down
Loading

0 comments on commit 89462db

Please sign in to comment.