[KOGITO-9177] Introduce workflowproj module #143

ricardozanini · 2023-05-30T21:09:53Z

Description of the change:
This adds a new module to the project, so client code can create a whole Kubernetes Workflow project from local files or using any golang tooling.

Export api package so users won't have to depend on the whole operator
Export the new workflowproj package, so users can programmatically create a new Kogito Workflow Kubernetes Deployment via the operator
Write integration tests
Document in the README (rewrite the whole thing)

See
https://issues.redhat.com/browse/KOGITO-9177
Motivation for the change:
Adds the capability to convert simple Kogito SW files into a new devmode deployment.

Checklist

Add or Modify a unit test for your change
Have you verified that all the tests are passing?

How to backport a pull request to a different branch?

In order to automatically create a backporting pull request please add one or more labels having the following format backport-<branch-name>, where <branch-name> is the name of the branch where the pull request must be backported to (e.g., backport-7.67.x to backport the original PR to the 7.67.x branch).

NOTE: backporting is an action aiming to move a change (usually a commit) from a branch (usually the main one) to another one, which is generally referring to a still maintained release branch. Keeping it simple: it is about to move a specific change or a set of them from one branch to another.

Once the original pull request is successfully merged, the automated action will create one backporting pull request per each label (with the previous format) that has been added.

If something goes wrong, the author will be notified and at this point a manual backporting is needed.

NOTE: this automated backporting is triggered whenever a pull request on main branch is labeled or closed, but both conditions must be satisfied to get the new PR created.

ricardozanini · 2023-05-31T20:50:07Z

.github/workflows/e2e.yml

@@ -104,6 +105,7 @@ jobs:
          kubectl version
          kubectl get pods -A

+      # TODO: install the operator-sdk first, then cache the installation
      - name: Run tests


https://issues.redhat.com/browse/KOGITO-9264

radtriste

Here are a few comments.
Gonna test this tomorrow morning

README.md

radtriste · 2023-06-01T18:51:01Z

test/samples/kogitoserverlessworkflow_generation1.yaml

Why this filename change ?

it's a test case, I'll move it to testdata once @desmax74's PR is rebased and merged.

workflowproj/resources_test.go

workflowproj/workflowproj_test.go

desmax74

Looks good, added few questions

desmax74 · 2023-06-01T18:49:10Z

controllers/builder/openshiftbuilder.go

+// In dev mode means within the src/main/resources. In build contexts, the actual context dir.
+var externalResourceDestinationDir = map[metadata.ExtResType]string{
+	metadata.ExtResGeneric:  "",
+	metadata.ExtResCamel:    workflowdef.ExternalResourceCamelMountDir,


It's only for Camel or is just the first extension to be used and tested ?

I don't understand your question. This map is to map (hehehe) the resource type and the target mount path. Except for camel, everything is mounted in src/main/resources, which is a bug I intend to fix here: https://issues.redhat.com/browse/KOGITO-9263

desmax74 · 2023-06-01T18:57:32Z

workflowproj/camelschema.go

+
+package workflowproj
+
+// camelSchema version 3.20.5. We can update this schema as we go, or add support to more than one in the future.


Is the only way to load this schema ?

We could fetch from the internet, but CLIs that can't access external resources would fail to load. It's a fair question. If you have another idea, we could try in another PR, maybe?

@ricardozanini
In case of updates of the remote camel's file we have to change this file, isn't better to have a copy of this file external to the code, or put the content on a template file and load at the startup when is needed ? An external file is simple to edit and save instead with this approach we need to compile and redeploy an entire operator, image if only this file change, and we have to redeploy on the catalog/hub a new version/csv/olm/other configs files files only for a single line change on this. In the future this could be a separate module and the operator at the startup load the latest version without internal changes

can't we delegate this validation to the engine or to the quarkus-camel extension(idk if the extension validate is as well.)?

isn't better to have a copy of this file external to the code, or put the content on a template file and load at the startup when is needed

In this use case, it's not. Usually we don't pack resources/templates in golang projects. Yet, this will be used by a CLI application. I'd say that's way better to align the Camel version upgrade on runtimes and also do it here using CI/automation instead.

Also, this is a corner case where we try to guess the contents of a file. It's not strict to the project, also it will change once I implement https://issues.redhat.com/browse/KOGITO-9263

image if only this file change, and we have to redeploy on the catalog/hub a new version/csv/olm/other configs files files only for a single line change on this

We will only change this if Kogito changes its Camel version, which require a new build/version of the whole platform. This will be just yet another change.

can't we delegate this validation to the engine or to the quarkus-camel extension(idk if the extension validate is as well.)?

Using quarkus/java here?

workflowproj/testdata/mygeneric.wsdl

workflowproj/testdata/valid-asyncapi.json

workflowproj/testdata/valid-asyncapi.yaml

workflowproj/testdata/workflows/specs/workflow-service-schema.json

workflowproj/testdata/workflows/workflow-minimal.sw.json

workflowproj/testdata/workflows/workflow-service.sw.json

README.md

desmax74 · 2023-06-01T19:08:26Z

api/v1alpha08/kogitoserverlessworkflow_types.go

@@ -33,8 +33,7 @@ type KogitoServerlessWorkflowSpec struct {
 type KogitoServerlessWorkflowStatus struct {
 	api.Status `json:",inline"`
 	// +optional
-	Address duckv1.Addressable           `json:"address,omitempty"`
-	Applied KogitoServerlessWorkflowSpec `json:"applied,omitempty"`


Isn't more needed ?

Applied should've been removed in your PR: https://github.com/kiegroup/kogito-serverless-operator/pull/128

desmax74 · 2023-06-01T19:13:12Z

controllers/kogitoserverlessbuild_controller_test.go

@@ -37,6 +37,7 @@ func TestKogitoServerlessBuildController(t *testing.T) {
 		WithRuntimeObjects(ksb, ksw).
 		WithRuntimeObjects(test.GetKogitoServerlessPlatformInReadyPhase("../config/samples/"+test.KogitoServerlessPlatformWithCacheYamlCR, namespace)).
 		WithRuntimeObjects(test.GetKogitoServerlessOperatorBuilderConfig("../", namespace)).
+		WithStatusSubresource(ksb, ksw).


Why WithStatusSubresource istead of WithStatusSubResource ?

This is a signature of the controller-runtime package: kubernetes-sigs/controller-runtime#2259

Signed-off-by: Ricardo Zanini <zanini@redhat.com>

controllers/kogitoserverlessbuild_controller.go

docs/CONTRIBUTING.md

ederign · 2023-06-05T17:58:10Z

workflowproj/workflowproj.go

+	if err := w.parseRawProject(); err != nil {
+		return err
+	}
+	if err := saveAsKubernetesManifest(w.project.Workflow, path); err != nil {


Check my comment on the PR

docs/CONTRIBUTING.md

test/samples/kogitoserverlessworkflow_generation1.yaml

ederign · 2023-06-05T18:01:33Z

@ricardozanini thank you so much for this PR. One small change that I would like to include:

We need to add null check if there is only the single file and no resources/app properties:

My suggestion:

workflowproj.go


func (w *workflowProjectHandler) SaveAsKubernetesManifests(path string) error {
   if err := ensurePath(path); err != nil {
   	return err
   }
   if err := w.parseRawProject(); err != nil {
   	return err
   }
   if err := saveAsKubernetesManifest(w.project.Workflow, path); err != nil {
   	return err
   }
//new check
   if w.project.Properties != nil {
   	if err := saveAsKubernetesManifest(w.project.Properties, path); err != nil {
   		return err
   	}
   }
//new check
   if w.project.Resources != nil {
   	for _, r := range w.project.Resources {
   		if err := saveAsKubernetesManifest(r, path); err != nil {
   			return err
   		}
   	}
   }
   return nil
}

A test to catch that:

func Test_Handler_WorkflowService_SaveAs(t *testing.T) {
	handler := New("default").
		WithWorkflow(getWorkflowService())

	proj, err := handler.AsObjects()
	assert.NoError(t, err)
	assert.NotNil(t, proj.Workflow)

	tmpPath, err := os.MkdirTemp("", "*-test")
	assert.NoError(t, err)
	defer os.RemoveAll(tmpPath)

	assert.NoError(t, handler.SaveAsKubernetesManifests(tmpPath))
	files, err := os.ReadDir(tmpPath)
	assert.NoError(t, err)
	assert.Len(t, files, 1)

	for _, f := range files {
		if strings.HasSuffix(f.Name(), yamlFileExt) {
			contents, err := os.ReadFile(path.Join(tmpPath, f.Name()))
			assert.NoError(t, err)
			decode := scheme.Codecs.UniversalDeserializer().Decode
			k8sObj, _, err := decode(contents, nil, nil)
			assert.NoError(t, err)
			assert.NotNil(t, k8sObj)
			assert.NotEmpty(t, k8sObj.GetObjectKind().GroupVersionKind().String())
		}
	}

}

ederign

Just to make sure that @ricardozanini address my comment before someone merge it.

workflowproj/io.go

workflowproj/operator.go

spolti

small comments, not mandatory.
Many thanks.

Signed-off-by: Ricardo Zanini <zanini@redhat.com>

ederign

🚀 I'll keep you posted if I need anything else! Thank you so much

ricardozanini · 2023-06-05T19:26:37Z

@davidesalerno since we are in a hurry to integrate with the CLI, I'm merging this one. If you spot anything that requires our attention, let me know so I can make the required adjustments.

ricardozanini requested review from davidesalerno, spolti and radtriste May 30, 2023 21:23

desmax74 self-requested a review May 31, 2023 09:00

ricardozanini marked this pull request as ready for review May 31, 2023 20:03

ricardozanini commented May 31, 2023

View reviewed changes

ricardozanini mentioned this pull request May 31, 2023

[KOGITO-9260] Add the kubectl version to avoid issues on the create objects on the cluster #146

Closed

2 tasks

radtriste reviewed Jun 1, 2023

View reviewed changes

desmax74 reviewed Jun 1, 2023

View reviewed changes

ricardozanini mentioned this pull request Jun 1, 2023

[KOGITO-8961][KSW-Operator] Split use case and test examples #139

Merged

2 tasks

ricardozanini added 2 commits June 2, 2023 09:27

[KOGITO-9177] Introduce workflowproj module

ef174a7

Signed-off-by: Ricardo Zanini <zanini@redhat.com>

Fix unit test with subresource

8b5e4cd

Signed-off-by: Ricardo Zanini <zanini@redhat.com>

ricardozanini force-pushed the kogito-9177 branch from 17c1a7c to 8b5e4cd Compare June 2, 2023 14:19

radtriste approved these changes Jun 5, 2023

View reviewed changes