Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #253: JKubeServiceHub BuildService implementations are now loaded via PluginServiceFactory #772

Merged
merged 3 commits into from
Jul 16, 2021

Conversation

manusa
Copy link
Member

@manusa manusa commented Jul 8, 2021

Description

Supersedes closes #748
Fixes #253

Complete refactor of the BuildService(s) and load process in JKubeServiceHub

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change
  • Chore (non-breaking change which doesn't affect codebase;
    test, version modification, documentation, etc.)

Checklist

…on mechanism

+ Use ServiceLoader to load all Build Services and pick up service
  which is applicable in current context. This requires BuildService
  implementations to have zero-arg constructors so I need to set the
  elements from setters instead.

+ Added isApplicable(), setJKubeServiceHub(), methods in
  BuildService interface

Signed-off-by: Rohan Kumar <[email protected]>
@manusa manusa force-pushed the review/build-service-SPI branch from 7ed2489 to 140e7b3 Compare July 8, 2021 15:19
@manusa manusa marked this pull request as draft July 8, 2021 15:53
@manusa manusa force-pushed the review/build-service-SPI branch 2 times, most recently from 5eff5a8 to 8db3631 Compare July 8, 2021 16:04
- Refactor OpenshiftBuildService
- Improve coverage
- Add specific tests for build service election order

or the different Quarkus packaging modes

Signed-off-by: Marc Nuri <[email protected]>
@manusa manusa force-pushed the review/build-service-SPI branch from 8db3631 to 4854f43 Compare July 8, 2021 16:12
@codecov
Copy link

codecov bot commented Jul 8, 2021

Codecov Report

Merging #772 (e1cf1ab) into master (acd5106) will increase coverage by 0.15%.
The diff coverage is 85.08%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #772      +/-   ##
============================================
+ Coverage     46.21%   46.36%   +0.15%     
- Complexity     3246     3261      +15     
============================================
  Files           413      415       +2     
  Lines         19856    19852       -4     
  Branches       2781     2778       -3     
============================================
+ Hits           9176     9204      +28     
+ Misses         9661     9633      -28     
+ Partials       1019     1015       -4     
Impacted Files Coverage Δ
...t/config/service/openshift/ImageStreamService.java 57.02% <ø> (ø)
...lipse/jkube/maven/plugin/mojo/build/BuildMojo.java 8.33% <0.00%> (ø)
...onfig/service/openshift/OpenshiftBuildService.java 49.01% <71.05%> (-3.99%) ⬇️
.../service/openshift/OpenShiftBuildServiceUtils.java 86.59% <86.59%> (ø)
.../eclipse/jkube/kit/build/api/helper/BuildUtil.java 91.66% <87.50%> (-8.34%) ⬇️
.../config/service/kubernetes/DockerBuildService.java 78.94% <88.88%> (+23.94%) ⬆️
...se/jkube/kit/common/util/PluginServiceFactory.java 87.50% <100.00%> (+5.27%) ⬆️
.../jkube/kit/config/service/BuildServiceManager.java 100.00% <100.00%> (ø)
...ipse/jkube/kit/config/service/JKubeServiceHub.java 90.47% <100.00%> (+1.74%) ⬆️
...kit/config/service/kubernetes/JibBuildService.java 92.42% <100.00%> (ø)
... and 3 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 acd5106...e1cf1ab. Read the comment docs.

@manusa manusa marked this pull request as ready for review July 9, 2021 04:27
@manusa manusa requested a review from rohanKanojia July 12, 2021 10:04
Reuse the PluginServiceFactory that provides injection for Generators
and Enrichers.
Order of BuildService can be declared using our internal syntax
in the service definition files.

or the different Quarkus packaging modes

Signed-off-by: Marc Nuri <[email protected]>
@manusa manusa force-pushed the review/build-service-SPI branch from 4854f43 to e1cf1ab Compare July 16, 2021 08:18
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

87.0% 87.0% Coverage
0.0% 0.0% Duplication

@manusa manusa added this to the 1.4.0 milestone Jul 16, 2021
@manusa manusa merged commit 96c7068 into eclipse-jkube:master Jul 16, 2021
@manusa manusa deleted the review/build-service-SPI branch July 21, 2021 04:40
clusterAccess = new ClusterAccess(log,
ClusterConfiguration.from(System.getProperties(), jKubeServiceHub.getConfiguration().getProject().getProperties()).build());
}
client = clusterAccess.createDefaultClient();
Copy link
Member

@rohanKanojia rohanKanojia Aug 4, 2021

Choose a reason for hiding this comment

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

assigning value of clusterAccess.createDefaultClient() which can either be DefaultKubernetesClient or DefaultOpenShiftClient seems to OpenShiftClient client seems to be causing #815

In v1.3.0, this code was in JKubeServiceHub and client was of type KubernetesClient
https://github.com/eclipse/jkube/blob/v1.3.0/jkube-kit/config/service/src/main/java/org/eclipse/jkube/kit/config/service/JKubeServiceHub.java#L62

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor JKubeServiceHub's BuildService election mechanism
2 participants