refactor (jkube-kit/config) : Move OpenShift S2I Build configuration fields from BuildServiceConfig to Image's BuildConfiguration #2378
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Eclipse JKube CI ReportStarted new GH workflow run for #2378 (2024-04-25T13:41:36Z) ⚙️ JKube E2E Tests (8833496308)
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2378 +/- ##
=============================================
+ Coverage 59.36% 70.58% +11.22%
- Complexity 4586 5027 +441
=============================================
Files 500 487 -13
Lines 21211 19471 -1740
Branches 2830 2511 -319
=============================================
+ Hits 12591 13743 +1152
+ Misses 7370 4504 -2866
+ Partials 1250 1224 -26 ☔ View full report in Codecov by Sentry. |
rohanKanojia
force-pushed
the
pr/buildserviceconfig-to-buildconfig
branch
2 times, most recently
from
dc07393 to
743d426
Compare
|
Kudos, SonarCloud Quality Gate passed!
|
rohanKanojia
force-pushed
the
pr/buildserviceconfig-to-buildconfig
branch
2 times, most recently
from
0806a0e to
9a2f15d
Compare
rohanKanojia
changed the title
String/boolean fields from BuildServiceConfig to Image's BuildConfiguration
rohanKanojia
force-pushed
the
pr/buildserviceconfig-to-buildconfig
branch
from
9a2f15d to
e953f56
Compare
rohanKanojia
changed the title
String/boolean fields from BuildServiceConfig to Image's BuildConfiguration
rohanKanojia
changed the title
rohanKanojia
force-pushed
the
pr/buildserviceconfig-to-buildconfig
branch
5 times, most recently
from
41e14c6 to
5a52803
Compare
rohanKanojia
force-pushed
the
pr/buildserviceconfig-to-buildconfig
branch
4 times, most recently
from
473330c to
9329701
Compare
manusa
reviewed
Feb 28, 2024
| @@ -220,6 +221,8 @@ protected File getManifest(KubernetesClient kc) { | |||
| return manifest; | |||
| } | |||
|
|
|||
| protected void postProcessResolvedImages() { } | |||
There was a problem hiding this comment.
we'll need to discuss this again, the idea we discussed was not to post-process the resulting images, but to make sure that the configuration was stable (and hopefully immutable) before propagating it downstream.
There was a problem hiding this comment.
Actually, I had added this before the ConfigHelper.initImageConfiguration but I noticed that generators are invoked inside this call and resolveImages get populated after this invocation (for zero configuration scenarios).
|
Reminder.
|
This was referenced Mar 4, 2024
rohanKanojia
force-pushed
the
pr/buildserviceconfig-to-buildconfig
branch
3 times, most recently
from
4f2416f to
3922834
Compare
rohanKanojia
force-pushed
the
pr/buildserviceconfig-to-buildconfig
branch
2 times, most recently
from
11ad2b1 to
55c82e1
Compare
|
rohanKanojia
force-pushed
the
pr/buildserviceconfig-to-buildconfig
branch
from
55c82e1 to
b9e2c23
Compare
rohanKanojia
force-pushed
the
pr/buildserviceconfig-to-buildconfig
branch
2 times, most recently
from
1713230 to
48fad50
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2378 +/- ##
=============================================
+ Coverage 59.36% 70.72% +11.36%
- Complexity 4586 5073 +487
=============================================
Files 500 489 -11
Lines 21211 19574 -1637
Branches 2830 2526 -304
=============================================
+ Hits 12591 13844 +1253
+ Misses 7370 4501 -2869
+ Partials 1250 1229 -21 ☔ View full report in Codecov by Sentry. |
rohanKanojia
force-pushed
the
pr/buildserviceconfig-to-buildconfig
branch
from
48fad50 to
782eb44
Compare
manusa
reviewed
Apr 25, 2024
| * <p> | ||
| * This field is applicable in case of OpenShift <code>s2i</code> build strategy | ||
| */ | ||
| private String openshiftBuildRecreateMode; |
There was a problem hiding this comment.
Why is this a string here instead of an enum as it was before?
There was a problem hiding this comment.
enum BuildRecreateMode is in jkube-kit/config/resource, it's not available in jkube-kit/config/image module.
Shall I move this enum to jkube-kit/common?
…Config to Image's BuildConfiguration Signed-off-by: Rohan Kumar <[email protected]>
Signed-off-by: Rohan Kumar <[email protected]>
rohanKanojia
force-pushed
the
pr/buildserviceconfig-to-buildconfig
branch
from
782eb44 to
9998085
Compare
manusa
reviewed
Apr 25, 2024
Comment on lines
+63
to
+64
| BuildConfiguration updatedBuildConfig = mergeGlobalConfigParamsWithSingleImageBuildConfig(imageConfiguration.getBuild()); | ||
| imageConfiguration.setBuild(updatedBuildConfig); |
There was a problem hiding this comment.
In a future iteration we'll need to see if we can refactor ImageConfiguration.initAndValidate method to take care of this.
Signed-off-by: Marc Nuri <[email protected]>
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
ConfigHelper.initImageConfigurationto a dedicated generator #2802 refactor (jkube-kit) : MoveConfigHelper.initImageConfigurationtoGeneratorManager.generateAndMerge#2818 to be merged firstbuildRecreateMode->openshiftBuildRecreateModeforcePull->openshiftForcePulls2iBuildNameSuffix->openshiftS2iBuildNameSuffixs2iImageStreamLookupPolicyLocal->openshiftS2iImageStreamLookupPolicyLocalopenshiftPullSecret->openshiftPullSecretopenshiftPushSecret->openshiftPushSecretbuildOutputKind->openshiftBuildOutputKindThese fields are for configuring the OpenShift S2I build process. These fields make more sense to be in BuildConfiguration as we want our BuildConfiguration to accommodate fields for build strategies other than docker.
With this change, we're exposing the abovementioned configuration fields from these sources:
If both configuration options are provided, priority would be given to options provided inside
image>buildconfigurationType of change
test, version modification, documentation, etc.)
Checklist