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

[KOGITO-3296] - Upgrading Infinispan CRD and fixing custom credential… #555

Merged
merged 2 commits into from Sep 14, 2020

Conversation

ricardozanini
Copy link
Member

…s bug

See: https://issues.redhat.com/browse/KOGITO-3296

In this PR we upgrade the Infinispan CRDs, so now users can use the 2.x channel from the OLM installation. Also, the generated secret from Infinispan is now attached to our secret, which will be injected into the KogitoService.

I made a clean up in the KogitoInfra controller package, which will help KOGITO-3039 as well.

@r00ta please review your reproducer, the identities.yaml file had the wrong name. To add a custom credential to a KogitoService, you only have to create a custom Infinispan credential secret, add it to the Infinispan CR and make your KogitoService to use the infra provided by us. If you would create your own Infinispan CR, just add the custom credentials there the same way you did, create a new secret for the KogitoService with the same credentials but with different format and add everything manually to the KogitoService. Is not the ideal, but KOGITO-3039 will fix that. You would create your own Infinispan and tell the Kogito Operator that you would like to use that reference, every thing will be done for you.

cc @vaibhavjainwiz

@sutaakar I didn't make great changes in this PR, I think would be a nice addition to 0.15 so people won't have problems with the Infinispan CRD any more.

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

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

…s bug

Signed-off-by: Ricardo Zanini <zanini@redhat.com>
@ricardozanini ricardozanini added this to the v0.15.0 milestone Sep 11, 2020
@kie-ci-bot kie-ci-bot bot added bdd-tests 🧪 PR is related to the BDD test framework needs review 🔍 Pull Request that needs reviewers operator ☁️ Pull Request related to kogito-operator code base labels Sep 11, 2020
@ricardozanini ricardozanini removed the request for review from radtriste September 11, 2020 20:51
@codecov
Copy link

codecov bot commented Sep 11, 2020

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #555      +/-   ##
==========================================
- Coverage   42.01%   37.43%   -4.58%     
==========================================
  Files         169      160       -9     
  Lines        9012     7915    -1097     
==========================================
- Hits         3786     2963     -823     
+ Misses       4812     4606     -206     
+ Partials      414      346      -68     
Flag Coverage Δ
#cli 55.78% <ø> (+0.80%) ⬆️
#operator 31.24% <ø> (-7.34%) ⬇️

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

Impacted Files Coverage Δ
...d/kogito/command/converter/monitoring_converter.go 100.00% <ø> (ø)
...ogito/command/converter/webhooksecret_converter.go 80.00% <ø> (ø)
cmd/kogito/command/deploy/delete_service.go 78.94% <ø> (ø)
cmd/kogito/command/deploy/deploy_service.go 65.38% <ø> (+0.91%) ⬆️
cmd/kogito/command/flag/build_flag.go 0.00% <ø> (ø)
cmd/kogito/command/flag/monitoring_flag.go 0.00% <ø> (ø)
cmd/kogito/command/flag/runtime_flag.go 0.00% <ø> (ø)
cmd/kogito/command/flag/runtimetype_flag.go 0.00% <ø> (ø)
cmd/kogito/command/flag/webhook_flag.go 0.00% <ø> (ø)
cmd/kogito/command/install/factory.go 100.00% <ø> (ø)
... and 108 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 8654780...63ba954. Read the comment docs.

pkg/controller/kogitoinfra/infinispan/resource.go Outdated Show resolved Hide resolved
@@ -105,44 +82,33 @@ func CreateRequiredResources(kogitoInfra *v1alpha1.KogitoInfra, cli *client.Clie
}

func newInfinispanResource(kogitoInfra *v1alpha1.KogitoInfra) *infinispan.Infinispan {
infinispanRes := &infinispan.Infinispan{
return &infinispan.Infinispan{
Copy link
Contributor

Choose a reason for hiding this comment

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

Kindly verify if this function make more sence in infrastucture/infinispan

Copy link
Member Author

Choose a reason for hiding this comment

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

This is just to create the custom k8s resource (as we create a Deployment for example), it's not a role of the shared infrasctructure package. It meant to have shared functions among the controllers only. This package will be refactored soon and the Infinispan CR creation dropped.

@sutaakar
Copy link
Contributor

/jenkins test

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.

@ricardozanini
Copy link
Member Author

@sutaakar do you mind verifying this manually as well? :(

@sutaakar
Copy link
Contributor

sure, will try

@sutaakar
Copy link
Contributor

@ricardozanini Checked using external credentials and generated credentials for both Infinispan operator 1.1.x and 2.0.x - both works ok (for Infinispan 2.0.x the Data index crash as it is not yet compatible, which is expected). Credentials are passed from Infinispan secret to Kogito one.

@ricardozanini
Copy link
Member Author

MANY THANKS for this super verification, @sutaakar . I'm pushing @vaibhavjainwiz suggestions, and as soon as we merge #558 I think we can run this CI again.

Signed-off-by: Ricardo Zanini <zanini@redhat.com>
@kie-ci-bot
Copy link

kie-ci-bot bot commented Sep 14, 2020

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

@ricardozanini
Copy link
Member Author

/jenkins test

@ricardozanini
Copy link
Member Author

Keycloak only failure, merging.

@ricardozanini ricardozanini added ready 🚀 PR is ready to be merged and removed needs review 🔍 Pull Request that needs reviewers labels Sep 14, 2020
@ricardozanini ricardozanini merged commit 3b244d1 into apache:master Sep 14, 2020
@ricardozanini ricardozanini deleted the kogito-3296 branch September 14, 2020 21:17
ricardozanini added a commit that referenced this pull request Sep 16, 2020
* add explainability service

* fix tests

* add command for explainability

* add operator expl

* add explainability BDD

* fix tests

* update yaml example

* fix crds, rebase

* inject kogitoruntime route into kogitoexplainability, implement integration test

* fix getJSON error

* remove env var injection in expl service

* fix bats

* Update kogitoruntime_controller.go

* add explainability service remove operation

* Update kogitoexplainability.go

* Update pkg/apis/app/v1alpha1/kogitoexplainability_types.go

Co-authored-by: Ricardo Zanini <1538000+ricardozanini@users.noreply.github.com>

* rebase

* Update cmd/kogito/command/shared/install_services.go

Co-authored-by: Karel Suta <krlsuta@gmail.com>

* Update cmd/kogito/command/shared/install_services.go

Co-authored-by: Karel Suta <krlsuta@gmail.com>

* Update app.kiegroup.org_v1alpha1_kogitoexplainability_cr.yaml

* Update cmd/kogito/command/shared/install_services.go

Co-authored-by: Karel Suta <krlsuta@gmail.com>

* Update pkg/controller/kogitoexplainability/kogitoexplainability_controller.go

Co-authored-by: Karel Suta <krlsuta@gmail.com>

* Update test/features/install_explainability.feature

Co-authored-by: Karel Suta <krlsuta@gmail.com>

* Update install_explainability.feature

* Update test/steps/kogitoexplainability.go

Co-authored-by: Karel Suta <krlsuta@gmail.com>

* set kogito runtime external uri in explaination bdd test

* Update test/features/install_explainability.feature

Co-authored-by: Karel Suta <krlsuta@gmail.com>

* Update kogitoexplainability.go

* Update cr_to_cli.go

* Update cr_to_cli.go

* Update deployer.go

* Update install_explainability.feature

* Update create_kogito_service.go

* make test

* Update kogitoexplainability.go

* Update deployer.go

* Update deployer.go

* revert deployer changes

* fix response content explainability

* [KOGITO-3323] Tests: Update scenario description to be unique for every run (#554)

Signed-off-by: Karel Suta <ksuta@redhat.com>

* KOGITO-3176 - disable standalone trusty test due to kafka change (#558)

* Update install_trusty.feature

* disable test

* [KOGITO-3335] Tests: Split current tests to operator and integration (#562)

Signed-off-by: Karel Suta <ksuta@redhat.com>

* Update explainability.go

* [KOGITO-3296] - Upgrading Infinispan CRD and fixing custom credential… (#555)

* [KOGITO-3296] - Upgrading Infinispan CRD and fixing custom credentials bug

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

* Applied peer review changes (secret/comparator)

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

* KOGITO-3332: Check whether jobs service is ready before deploying service (#556)

* KOGITO-3181 - add trusty ui scorecard/controller/tests (#537)

* add trusty ui controller and tests

* add trusty and trusty-ui remove operation

* inject trusty route

* Update pkg/apis/app/v1alpha1/kogitotrustyui_types.go

Co-authored-by: Ricardo Zanini <1538000+ricardozanini@users.noreply.github.com>

* remove routes, fix typos

* make vet

* add bats tests

* add tests

* fix typo in bdd test

* fix

* Update test/steps/kogitotrustyui.go

Co-authored-by: Karel Suta <krlsuta@gmail.com>

* Update test/features/install_trustyui.feature

Co-authored-by: Karel Suta <krlsuta@gmail.com>

* Update test/features/install_trustyui.feature

Co-authored-by: Karel Suta <krlsuta@gmail.com>

* Update test/features/install_trustyui.feature

Co-authored-by: Karel Suta <krlsuta@gmail.com>

* Update kogitotrusty_ui_controller.go

* update comments

* update comments

Co-authored-by: Ricardo Zanini <1538000+ricardozanini@users.noreply.github.com>
Co-authored-by: Karel Suta <krlsuta@gmail.com>

* KOGITO-3340: Cucumber Tests: Projects that use Keycloak remains as Terminating (#564)

* rebase

* rebase

* Revert "KOGITO-3340: Cucumber Tests: Projects that use Keycloak remains as Terminating (#564)"

This reverts commit 3fa1d80.

* Revert "KOGITO-3332: Check whether jobs service is ready before deploying service (#556)"

This reverts commit 1438671.

* Revert "[KOGITO-3323] Tests: Update scenario description to be unique for every run (#554)"

This reverts commit f94946e.

* Revert "[KOGITO-3335] Tests: Split current tests to operator and integration (#562)"

This reverts commit 745f8d7.

* add logs

Co-authored-by: Ricardo Zanini <1538000+ricardozanini@users.noreply.github.com>
Co-authored-by: Karel Suta <krlsuta@gmail.com>
Co-authored-by: Karel Suta <ksuta@redhat.com>
Co-authored-by: Jose Carvajal <josecarvajalhilario@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bdd-tests 🧪 PR is related to the BDD test framework operator ☁️ Pull Request related to kogito-operator code base ready 🚀 PR is ready to be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants