Skip to content
This repository has been archived by the owner on Mar 14, 2024. It is now read-only.

[KOGITO-3502] - Sasl mechanism no longer set by Kogito operator #593

Merged
merged 3 commits into from
Oct 2, 2020
Merged

[KOGITO-3502] - Sasl mechanism no longer set by Kogito operator #593

merged 3 commits into from
Oct 2, 2020

Conversation

vaibhavjainwiz
Copy link
Contributor

@vaibhavjainwiz vaibhavjainwiz commented Oct 1, 2020 •
edited

Many thanks for submiting your Pull Request ❤️!

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

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 kie-ci-bot bot added needs review 🔍 Pull Request that needs reviewers operator ☁️ Pull Request related to kogito-operator code base labels Oct 1, 2020
@codecov
Copy link

codecov bot commented Oct 1, 2020 •
edited

Codecov Report

Merging #593 into master will decrease coverage by 6.89%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #593      +/-   ##
==========================================
- Coverage   42.01%   35.11%   -6.90%     
==========================================
  Files         169      145      -24     
  Lines        9012     6903    -2109     
==========================================
- Hits         3786     2424    -1362     
+ Misses       4812     4157     -655     
+ Partials      414      322      -92
Flag Coverage Δ
#cli 60.74% <ø> (+5.76%) ⬆️
#operator 27.43% <ø> (-11.15%) ⬇️

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

Impacted Files Coverage Δ
...ito/command/converter/binarybuildtype_converter.go 100.00% <ø> (ø)
...ogito/command/converter/infraresource_converter.go 100.00% <ø> (ø)
...ito/command/converter/kogitobuildtype_converter.go 77.77% <ø> (-2.23%) ⬇️
...d/kogito/command/converter/monitoring_converter.go 100.00% <ø> (ø)
cmd/kogito/command/converter/native_converter.go 88.23% <ø> (ø)
.../kogito/command/converter/runtimetype_converter.go 80.00% <ø> (-20.00%) ⬇️
...ogito/command/converter/webhooksecret_converter.go 75.00% <ø> (-5.00%) ⬇️
cmd/kogito/command/deploy/delete_service.go 78.12% <ø> (-0.83%) ⬇️
cmd/kogito/command/deploy/deploy_service.go 61.19% <ø> (-3.28%) ⬇️
cmd/kogito/command/install/data_index.go 90.56% <ø> (+7.70%) ⬆️
... and 206 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 183855c...9da2134. Read the comment docs.

ricardozanini
ricardozanini reviewed Oct 1, 2020
View reviewed changes
examples/process-quarkus-example-persistence.yaml
spec:
resource:
apiVersion: kafka.strimzi.io/v1beta1
kind: Kafka
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the name is not set we assume KogitoInfra's name?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if name is not provided under resource then for infinispan we assume kogito-infinispan and for kafka we assume kogito-kafka

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might need to add this in the docs since it´s not clear from the interface.

@ricardozanini
Copy link
Member

/jenkins test

sutaakar
sutaakar approved these changes Oct 1, 2020
View reviewed changes
Copy link
Contributor

@sutaakar sutaakar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, thanks

@kie-ci-bot
Copy link

kie-ci-bot bot commented Oct 1, 2020

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

@ricardozanini
Copy link
Member

/jenkins test

MarianMacik
MarianMacik approved these changes Oct 2, 2020
View reviewed changes
pkg/controller/kogitoinfra/infinispan/infinispan_properties.go
@@ -109,5 +113,7 @@ func getInfinispanAppProps(cli *client.Client, name string, namespace string) (m
appProps[propertiesInfinispanSpring[appPropInfinispanServerList]] = infinispanURI
appProps[propertiesInfinispanQuarkus[appPropInfinispanServerList]] = infinispanURI
}
appProps[propertiesInfinispanSpring[appPropInfinispanSaslMechanism]] = saslPlain
appProps[propertiesInfinispanQuarkus[appPropInfinispanSaslMechanism]] = saslPlain
return appProps, nil
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So right now we always add all properties (Spring Boot and Quarkus) to the config map, no matter which runtime is running?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, because the KogitoInfra can be reused by multiple services, both Quarkus and SpringBoot

@spolti spolti added ready 🚀 PR is ready to be merged and removed needs review 🔍 Pull Request that needs reviewers labels Oct 2, 2020
@spolti spolti modified the milestones: v0.16.0, v0.17.0 Oct 2, 2020
@ricardozanini ricardozanini merged commit 1501b05 into apache:master Oct 2, 2020
@ricardozanini ricardozanini modified the milestones: v0.17.0, v0.16.0 Oct 2, 2020
@vaibhavjainwiz vaibhavjainwiz deleted the KOGITO-3502 branch October 5, 2020 08:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ricardozanini ricardozanini ricardozanini approved these changes

@sutaakar sutaakar sutaakar approved these changes

@spolti spolti spolti approved these changes

@MarianMacik MarianMacik MarianMacik approved these changes

@radtriste radtriste radtriste approved these changes

Assignees
No one assigned
Labels
operator ☁️ Pull Request related to kogito-operator code base ready 🚀 PR is ready to be merged
Projects
None yet
Milestone
v0.16.0
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@vaibhavjainwiz @ricardozanini @spolti @sutaakar @MarianMacik @radtriste