fix (jkube-kit/enricher) : GitEnricher should implement enrich instead of create
#2562
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 #2562 (2024-01-16T16:08:46Z) ⚙️ JKube E2E Tests (7544208007)
|
rohanKanojia
force-pushed
the
pr/issue2500-git-enricher
branch
from
07d7021 to
902bf95
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #2562 +/- ##
=============================================
+ Coverage 59.36% 70.00% +10.64%
- Complexity 4586 4911 +325
=============================================
Files 500 474 -26
Lines 21211 19152 -2059
Branches 2830 2480 -350
=============================================
+ Hits 12591 13407 +816
+ Misses 7370 4529 -2841
+ Partials 1250 1216 -34 ☔ View full report in Codecov by Sentry. |
…ead of `create` GitEnricher should be executed in `enrich` phase so that it's invoked during OpenShift build. Signed-off-by: Rohan Kumar <rohaan@redhat.com>
rohanKanojia
force-pushed
the
pr/issue2500-git-enricher
branch
from
902bf95 to
cf65355
Compare
|
manusa
reviewed
Jan 17, 2024
Comment on lines
+199
to
+209
| arguments(new ServiceBuilder().withNewMetadata().endMetadata().build()), | ||
| arguments(new DeploymentBuilder().withNewMetadata().endMetadata().build()), | ||
| arguments(new DeploymentConfigBuilder().withNewMetadata().endMetadata().build()), | ||
| arguments(new ReplicaSetBuilder().withNewMetadata().endMetadata().build()), | ||
| arguments(new ReplicationControllerBuilder().withNewMetadata().endMetadata().build()), | ||
| arguments(new DaemonSetBuilder().withNewMetadata().endMetadata().build()), | ||
| arguments(new StatefulSetBuilder().withNewMetadata().endMetadata().build()), | ||
| arguments(new JobBuilder().withNewMetadata().endMetadata().build()), | ||
| arguments(new SecretBuilder().withNewMetadata().endMetadata().build()), | ||
| arguments(new ImageStreamBuilder().withNewMetadata().endMetadata().build()), | ||
| arguments(new BuildConfigBuilder().withNewMetadata().endMetadata().build()) |
There was a problem hiding this comment.
Note that due to the changes in GitEnricher, now you need to create the ObjectMetaBuilder entry.
I'm not sure that everytime the resources reach the enricher they will contain an ObjectMetaBuilder instance.
Are you sure about this? (consider also fragments and other corner cases)
There was a problem hiding this comment.
I'm not so sure. I checked with a fragment without metadata but I noticed we always seem to initialize metadata while reading fragment into HasMetadata:
https://github.com/eclipse/jkube/blob/e101e93ed4bfad99ce9549d85aa968225326839e/jkube-kit/enricher/api/src/main/java/org/eclipse/jkube/kit/enricher/api/util/KubernetesResourceFragments.java#L138-L139
There was a problem hiding this comment.
Using ObjectMetadataBuilder also adds git labels in .metadata of Pod Template, not sure whether we want to have git annotations there.
We have PodAnnotationEnricher for copying annotations from controller metadata to Pod template metadata
There was a problem hiding this comment.
In the case of resources generated via opinionated defaults, I checked various handlers in org.eclipse.jkube.kit.enricher.handler package and they all seem to be generating a non null ObjectMeta always:
https://github.com/eclipse/jkube/blob/e101e93ed4bfad99ce9549d85aa968225326839e/jkube-kit/enricher/api/src/main/java/org/eclipse/jkube/kit/enricher/handler/DeploymentHandler.java#L42
Once resources go through enricher's create phase, jkube-metadata is the first enricher in enrich phase where various visitors try to create metadata if it doesn't exist:
https://github.com/eclipse/jkube/blob/e101e93ed4bfad99ce9549d85aa968225326839e/jkube-kit/enricher/api/src/main/java/org/eclipse/jkube/kit/enricher/api/visitor/MetadataVisitor.java#L134
manusa
approved these changes
Jan 18, 2024
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
Related to #2500
GitEnricher should be executed in
enrichphase so that it's invoked during OpenShift build.enrichfromcreatemethod@BeforeEachBuildConfig,ImageStream,Secretresources to parameterized method sourceType of change
test, version modification, documentation, etc.)
Checklist