[KOGITO-2960] - Prometheus integration with kogito-cloud-operator #521

vaibhavjainwiz · 2020-08-18T05:25:46Z

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

Many thanks for submiting your Pull Request ❤️!

Please make sure that your PR meets the following requirements:

You have read the contributors' guide
Pull Request title is properly formatted: [KOGITO-XYZ] Subject
Pull Request contains link to the JIRA issue
Pull Request contains 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

kie-ci-bot · 2020-08-18T05:25:56Z

/jenkins test

codecov · 2020-08-18T05:28:35Z

Codecov Report

Merging #521 into master will decrease coverage by 4.40%.
The diff coverage is 19.61%.

@@            Coverage Diff             @@
##           master     #521      +/-   ##
==========================================
- Coverage   42.01%   37.60%   -4.41%     
==========================================
  Files         169      162       -7     
  Lines        9012     7933    -1079     
==========================================
- Hits         3786     2983     -803     
+ Misses       4812     4602     -210     
+ Partials      414      348      -66

Flag	Coverage Δ
#cli	`55.56% <61.34%> (+0.58%)`	⬆️
#operator	`31.60% <9.63%> (-6.98%)`	⬇️

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

Impacted Files	Coverage Δ
cmd/kogito/command/flag/build_flag.go	`0.00% <0.00%> (ø)`
cmd/kogito/command/flag/monitoring_flag.go	`0.00% <0.00%> (ø)`
cmd/kogito/command/flag/runtime_flag.go	`0.00% <0.00%> (ø)`
cmd/kogito/command/flag/runtimetype_flag.go	`0.00% <ø> (ø)`
cmd/kogito/command/flag/webhook_flag.go	`0.00% <ø> (ø)`
cmd/kogito/command/service/build_service.go	`0.00% <0.00%> (ø)`
cmd/kogito/command/service/runtime_service.go	`0.00% <0.00%> (ø)`
cmd/kogito/command/shared/install_services.go	`0.00% <0.00%> (ø)`
pkg/apis/app/v1alpha1/conditions.go	`89.47% <ø> (ø)`
pkg/apis/app/v1alpha1/kogitobuild_types.go	`20.00% <0.00%> (-80.00%)`	⬇️
... and 70 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 35f4b52...d611316. Read the comment docs.

ricardozanini

Thanks Jain! Looks good, I'm just curious about this "custom_writer/reader" that you created, mimicking what we already have on client package, but with this swtich case to verify if it's a Prometheus object or not. Why we need it? I believe we can think on something different to reuse the existing functions.

EDIT: I'm not seeing an update in the openapi generated file. Can you please confirm that you executed the openapi binary? Also, were you able to test this integration with the Prometheus Operator on minikube/crc?

...og/kogito-operator/1.0.0-snapshot/kogito-operator.v1.0.0-snapshot.clusterserviceversion.yaml

deploy/olm-catalog/kogito-operator/manifests/kogito-operator.clusterserviceversion.yaml

deploy/operator.yaml

pkg/controller/kogitoruntime/deployer.go

pkg/infrastructure/services/custom_reader.go

pkg/infrastructure/services/custom_writer.go

ricardozanini · 2020-08-18T10:55:06Z

/jenkins test

kie-ci-bot · 2020-08-18T11:25:09Z

Change detected in the PR, requesting reviews and running pipeline(if required) again

kie-ci-bot · 2020-08-18T11:25:16Z

/jenkins test

kie-ci-bot · 2020-08-18T11:39:21Z

Change detected in the PR, requesting reviews and running pipeline(if required) again

kie-ci-bot · 2020-08-18T11:39:28Z

/jenkins test

ricardozanini · 2020-08-18T11:51:07Z

/jenkins test

vaibhavjainwiz · 2020-08-19T02:57:34Z

Thanks Jain! Looks good, I'm just curious about this "custom_writer/reader" that you created, mimicking what we already have on client package, but with this swtich case to verify if it's a Prometheus object or not. Why we need it? I believe we can think on something different to reuse the existing functions.

@ricardozanini
ServiceMonitor is Prometheus operator object so to read/write its value we have to use prometheus client.
Currently we are following below approach :

Generate all requested resource
Use controller client reader to load all deployed resource
Use controller client writer to update all delta resource

Code is written to work with controller client only that why I had written custom reader/writer object which receive complete Client reference and at runtime decides which client(Controller/Prometheus) to use to load/update objects.

custom_reader calls when we say reader.ListAll(objectTypes...) under getDeployedResources() method
custom_writer call when we say writer.Add/Update/DeleteResources(delta.Added);

EDIT: I'm not seeing an update in the openapi generated file. Can you please confirm that you executed the openapi binary?

Following auto-generated files are included in change list :

app.kiegroup.org_kogitoruntimes_crd.yaml
app.kiegroup.org_kogitoruntimes_crd.yaml
kogito-operator.v1.0.0-snapshot.clusterserviceversion.yaml
app.kiegroup.org_kogitoruntimes_crd.yaml
kogito-operator.clusterserviceversion.yaml
zz_generated.deepcopy.go

Also, were you able to test this integration with the Prometheus Operator on minikube/crc?

I tried it on OC cluster. This PR has successfully generates ServiceMonitor object/stats under Prometheus Operator for all kogito-runtime which pass scrap flag as true.

vaibhavjainwiz · 2020-08-19T03:36:19Z

/jenkins test

kie-ci-bot · 2020-08-19T08:56:57Z

Change detected in the PR, requesting reviews and running pipeline(if required) again

kie-ci-bot · 2020-08-19T08:57:04Z

/jenkins test

ricardozanini · 2020-08-19T10:58:37Z

Code is written to work with controller client only that why I had written custom reader/writer object which receive complete
Client reference and at runtime decides which client(Controller/Prometheus) to use to load/update objects.

Hi!

My point is that this interface of "custom resources" that you created is basically this:

https://github.com/kiegroup/kogito-cloud-operator/blob/master/pkg/client/kubernetes/resource.go

Excepts that you are adding a condition of "if is prometheus resource". I'm suggesting to change this to have one interface doing the wrapper around the client that is smart enough to handle Prometheus resources as well.

pkg/apis/app/v1alpha1/kogitoruntime_types.go

deploy/olm-catalog/kogito-operator/manifests/kogito-operator.clusterserviceversion.yaml

ricardozanini · 2020-08-28T10:14:11Z

@spolti @sutaakar @Kaitou786 could you guys please take a look in this PR?

pkg/controller/kogitoruntime/deployer.go

pkg/client/prometheus/prometheus.go

pkg/controller/kogitoruntime/deployer.go

spolti

approving, but left a comment about the flags used.

cmd/kogito/command/flag/prometheus_flag.go

pkg/infrastructure/services/prometheus.go

[KOGITO-2960] - Incorporate review comments [KOGITO-2960] - Incorporate review comments [KOGITO-2960] - Incorporate review comments [KOGITO-2960] - Incorporate review comments [KOGITO-2960] - Incorporate review comments

ricardozanini

@vaibhavjainwiz I'm assuming you're going to update the "prometheus" name from the public interface to "monitoring" instead, right?

kie-ci-bot · 2020-09-01T19:21:47Z

Change detected in the PR, requesting reviews and running pipeline(if required) again

vaibhavjainwiz · 2020-09-01T19:22:43Z

@vaibhavjainwiz I'm assuming you're going to update the "prometheus" name from the public interface to "monitoring" instead, right?

yes..already done

kie-ci-bot · 2020-09-01T19:31:39Z

Change detected in the PR, requesting reviews and running pipeline(if required) again

cmd/kogito/command/flag/monitoring_flag.go

kie-ci-bot · 2020-09-02T05:42:14Z

Change detected in the PR, requesting reviews and running pipeline(if required) again

vaibhavjainwiz added 2 commits August 18, 2020 07:41

[KOGITO-2960] - Prometheus integration with kogito-cloud-operator

3039091

Merge branch 'master' into KOGITO-2960

ccc4aaf

kie-ci-bot bot requested review from ricardozanini and spolti August 18, 2020 05:25

kie-ci-bot bot added 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 Aug 18, 2020

vaibhavjainwiz requested review from radtriste, sutaakar and Kaitou786 August 18, 2020 05:26

ricardozanini suggested changes Aug 18, 2020

View reviewed changes

ricardozanini added this to the v0.15.0 milestone Aug 18, 2020

[KOGITO-2960] - Incorporate review comment

f6d66ca

Merge branch 'master' into KOGITO-2960

5de96e5

[KOGITO-2960] - Incorporate review comments

098d354

sutaakar reviewed Aug 19, 2020

View reviewed changes

pkg/apis/app/v1alpha1/kogitoruntime_types.go Outdated Show resolved Hide resolved

sutaakar reviewed Aug 19, 2020

View reviewed changes

deploy/olm-catalog/kogito-operator/manifests/kogito-operator.clusterserviceversion.yaml Outdated Show resolved Hide resolved

vaibhavjainwiz mentioned this pull request Aug 28, 2020

[KOGITO-2960] - Prometheus integration with kogito-cloud-operator apache/incubator-kie-docs#2792

Merged

ricardozanini removed the request for review from radtriste August 28, 2020 10:13

Kaitou786 approved these changes Aug 28, 2020

View reviewed changes