[KOGITO-4059] - Refactor Kogito operator into internal and core component #740

vaibhavjainwiz · 2021-01-28T18:01:47Z

Many thanks for submitting your Pull Request ❤️!

Jira issue : https://issues.redhat.com/browse/KOGITO-4059

Please make sure your PR meets the following requirements:

You have read the contributors' guide
Pull Request title is properly formatted: [KOGITO-XYZ] Subject
Pull Request contains a link to the JIRA issue
Pull Request contains a description of the issue
Pull Request does not include fixes for issues other than the main ticket
Your feature/bug fix has a unit test that verifies it
You've tested the new feature/bug fix in an actual OpenShift cluster
You've added a RELEASE_NOTES.md entry regarding this change

codecov · 2021-01-29T07:23:59Z

Codecov Report

Merging #740 (e341d31) into master (0b5d664) will increase coverage by 3.98%.
The diff coverage is 32.46%.

@@            Coverage Diff             @@
##           master     #740      +/-   ##
==========================================
+ Coverage   38.67%   42.65%   +3.98%     
==========================================
  Files         151      138      -13     
  Lines        6516     5256    -1260     
==========================================
- Hits         2520     2242     -278     
+ Misses       3601     2704     -897     
+ Partials      395      310      -85

Flag	Coverage Δ
cli	`52.57% <28.22%> (-1.74%)`	⬇️
controllers	`36.80% <36.80%> (?)`
operator	`39.83% <30.92%> (+5.36%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
cmd/kogito/command/converter/artifact_converter.go	`100.00% <ø> (ø)`
cmd/kogito/command/converter/config_converter.go	`4.76% <0.00%> (ø)`
cmd/kogito/command/converter/envvar_converter.go	`100.00% <ø> (ø)`
...md/kogito/command/converter/gitsource_converter.go	`100.00% <ø> (ø)`
...ogito/command/converter/infraresource_converter.go	`100.00% <ø> (ø)`
...d/kogito/command/converter/monitoring_converter.go	`100.00% <ø> (ø)`
...d/kogito/command/converter/properties_converter.go	`0.00% <0.00%> (ø)`
cmd/kogito/command/install/factory.go	`100.00% <ø> (ø)`
cmd/kogito/command/project/delete_project.go	`83.33% <ø> (ø)`
cmd/kogito/command/project/project.go	`100.00% <ø> (ø)`
... and 214 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 301da6c...d7b7b93. Read the comment docs.

controllers/kogitobuild_controller.go

controllers/kogitoinfra_controller_test.go

controllers/kogitoruntime_controller_test.go

core/connector/protobuf_test.go

core/kogitoinfra/knative_reconciler.go

core/kogitoservice/configmap.go

core/kogitoservice/deployment.go

core/kogitoservice/volume.go

core/kogitosupportingservice/supportingservice_reconciler.go

core/manager/infra_manager.go

core/manager/infra_manager_test.go

core/manager/runtime_manager.go

core/manager/supportingservice_manager.go

core/test/api/kogitobuild_types.go

sutaakar · 2021-02-01T14:40:03Z

Few minor functionality and missing header comments. Not sure why the headers weren't autogenerated.
The functionality looks ok on a brief look, I suppose the code was mostly just relocated.
Will try to run the operator locally and try some BDD tests.

sutaakar · 2021-02-18T18:40:15Z

For Kafka KogitoInfra there is a problem too, in quarkus aplication properties there is a springboot value for Kafka URI.
It seems related to GetRuntimeProperties() in api/v1beta1/kogitoinfra_types.go but I wasn't able to fix it properly so far. Feel free to fix it.

vaibhavjainwiz · 2021-02-19T07:59:12Z

For Kafka KogitoInfra there is a problem too, in quarkus aplication properties there is a springboot value for Kafka URI.
It seems related to GetRuntimeProperties() in api/v1beta1/kogitoinfra_types.go but I wasn't able to fix it properly so far. Feel free to fix it.

Its working now

kogito-operator.yaml

api/v1beta1/kogitoinfra_types.go

sutaakar · 2021-02-19T13:51:29Z

/jenkins test

sutaakar · 2021-02-19T15:33:22Z

The smoke tests pass locally.
Pass in a way that the issues found are not related to this PR and could be addressed separately.
Full test coverage can be ran once PR is merged.

sutaakar · 2021-02-22T12:41:49Z

/jenkins test

ricardozanini · 2021-02-22T12:44:05Z

Pass in a way that the issues found are not related to this PR and could be addressed separately.

Thanks, @sutaakar. Could you please open JIRAs to follow up on this one, and tackle these problems?

sutaakar · 2021-02-22T12:47:37Z

@ricardozanini it seems that those issues were related to changes in examples, I cannot reproduce it today.

ricardozanini · 2021-02-22T12:49:23Z

@radtriste @Kaitou786 can you please review?

sutaakar · 2021-02-22T13:51:36Z

/jenkins test

sutaakar · 2021-02-22T13:53:42Z

previous tests were crashing because there was an old Kogito operator deployed in CI, meddling with our tests.

sutaakar · 2021-02-22T14:38:59Z

core/kogitoservice/deployer_resources.go

 		if err != nil {
 			return nil, nil, nil, err
 		}

 		runtime := s.instance.GetSpec().GetRuntime()

 		// fetch app properties from Kogito infra instance
-		appProp := kogitoInfraInstance.Status.RuntimeProperties[runtime].AppProps
+		appProp := kogitoInfraInstance.GetStatus().GetRuntimeProperties()[runtime].GetAppProps()


Getting nil pointer error when on this line when deploying KogitoRuntime with KogitoInfra referencing Knative Broker:

E0222 14:32:53.431166 1 runtime.go:78] Observed a panic: "invalid memory address or nil pointer dereference" (runtime error: invalid memory address or nil pointer dereference) goroutine 600 [running]: k8s.io/apimachinery/pkg/util/runtime.logPanic(0x18b5d40, 0x2bd9730) /go/pkg/mod/k8s.io/apimachinery@v0.18.8/pkg/util/runtime/runtime.go:74 +0xa3 k8s.io/apimachinery/pkg/util/runtime.HandleCrash(0x0, 0x0, 0x0) /go/pkg/mod/k8s.io/apimachinery@v0.18.8/pkg/util/runtime/runtime.go:48 +0x82 panic(0x18b5d40, 0x2bd9730) /usr/local/go/src/runtime/panic.go:969 +0x166 github.com/kiegroup/kogito-cloud-operator/core/kogitoservice.(*serviceDeployer).fetchKogitoInfraProperties(0xc0007dc0e0, 0x1af6b2c, 0x1d, 0x0, 0x0, 0x0, 0x22, 0x1adbcc4, 0x6, 0xc000c2be20) /workspace/core/kogitoservice/deployer_resources.go:264 +0x3b4 github.com/kiegroup/kogito-cloud-operator/core/kogitoservice.(*serviceDeployer).createRequiredResources(0xc0007dc0e0, 0x0, 0x0, 0x1)

reported as https://issues.redhat.com/browse/KOGITO-4529

sutaakar · 2021-02-22T14:54:12Z

Test failures caused by https://github.com/kiegroup/kogito-examples/pull/582

sutaakar · 2021-02-22T15:12:24Z

Merged, retriggering PR check
/jenkins test

radtriste

I am good with the infrastructure change (could not check the go code but @sutaakar did it)

sutaakar · 2021-02-22T16:24:58Z

Test failure caused by missing quarkus-smallrye-health artifact in archetype.
🌄

Vaibhav added 2 commits January 28, 2021 23:27

KOGITO-4059 - Refactor Kogito operator into internal and core component

94999f0

Update release note

e622e8b

kie-ci-bot bot requested review from radtriste, ricardozanini, spolti and sutaakar January 28, 2021 18:01

kie-ci-bot bot added bdd-tests 🧪 PR is related to the BDD test framework cli 💻 Pull Request related to cli code base needs review 🔍 Pull Request that needs reviewers operator ☁️ Pull Request related to kogito-operator code base labels Jan 28, 2021

vaibhavjainwiz requested a review from Kaitou786 January 29, 2021 04:05

Vaibhav added 2 commits January 29, 2021 11:56

Fix test case

bca45b7

fix Version file path

5b9f09c

ricardozanini mentioned this pull request Jan 29, 2021

[KOGITO-3979] - Switch to current testing suite by OLM #739

Merged

8 tasks