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.

Sign up for GitHub

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

Jump to bottom

RemoveBuildAnnotationsEnricher removal #1263

Closed
3 of 4 tasks
manusa opened this issue Feb 2, 2022 · 3 comments · Fixed by #1412
Closed
3 of 4 tasks

RemoveBuildAnnotationsEnricher removal #1263

manusa opened this issue Feb 2, 2022 · 3 comments · Fixed by #1412
Assignees
@rohanKanojia
Labels
technical-debt
Milestone
1.8.0

Comments

@manusa
Copy link
Member

manusa commented Feb 2, 2022 •
edited

Description

The RemoveBuildAnnotationsEnricher seems to be completely outdated.

We should analyze what its initial purpose was. If it has nothing to do with Eclipse JKube, remove it.

AC

  • Remove class
  • Remove from enricher-default service definitions
  • Remove from profiles
  • Remove from docs
@manusa manusa added this to the 1.7.0 milestone Feb 2, 2022
@Hrushi20
Copy link

Should I remove the RemoveBuildAnnotationsEnricher class?

@manusa manusa modified the milestones: 1.7.0, 1.x Feb 23, 2022
@rohanKanojia rohanKanojia self-assigned this Mar 29, 2022
@rohanKanojia
Copy link
Member

RemoveBuildAnnotationsEnricher seems to be removing maven.jkube.io/source-url annotation from all resources present in KubernetesListBuilder.

This maven.jkube.io/source-url annotation seems to be getting added to all resources added via dependency in Dependency enricher:

annotations:
    maven.jkube.io/source-url: jar:file:/home/rokumar/work/repos/jkube/gradle-plugin/it/src/it/dependency-resources/dependency/build/libs/dependency-0.0.1-SNAPSHOT.jar!/META-INF/jkube/kubernetes.yml

Looks like this annotation is just used for some log statements which are printed during mergeResource phase:

https://github.com/eclipse/jkube/blob/479858aa6719ffaabe8320c87ec43d129c78ffe6/jkube-kit/enricher/api/src/main/java/org/eclipse/jkube/kit/enricher/api/util/KubernetesResourceUtil.java#L830
https://github.com/eclipse/jkube/blob/479858aa6719ffaabe8320c87ec43d129c78ffe6/jkube-kit/enricher/api/src/main/java/org/eclipse/jkube/kit/enricher/api/util/KubernetesResourceUtil.java#L762-L763

Maybe we can remove these log statements and usage of maven.jkube.io/source-url. Or maybe we can move annotation removal logic from enricher to DependencyEnricher

@rohanKanojia
Copy link
Member

I think RemoveBuildAnnotationsEnricher's annotation removal logic also needs some improvement.

https://github.com/eclipse/jkube/blob/bc2a7c809087451bb49b1a62299c0bdedacbea8b/jkube-kit/enricher/generic/src/main/java/org/eclipse/jkube/enricher/generic/RemoveBuildAnnotationsEnricher.java#L47-L54

It is currently removing maven.jkube.io/source-url annotation only from .metadata. But after this annotation is added to the resource in DependencyEnricher, PodAnnotationEnricher kicks in and copies this annotation to .spec.template.metadata. Due to this, we have leftovers in resources like this:
https://github.com/eclipse/jkube/blob/479858aa6719ffaabe8320c87ec43d129c78ffe6/kubernetes-maven-plugin/it/src/it/dependency-resources/expected/kubernetes.yml#L76-L77

rohanKanojia added a commit to rohanKanojia/jkube that referenced this issue Mar 30, 2022
@rohanKanojia
refactor (jkube-kit) : Remove RemoveBuildAnnotationsEnricher (eclipse…
7639351 
…-jkube#1263)

RemoveBuildAnnotationsEnricher seems to be removing
`maven.jkube.io/source-url` annotation which gets added in
DependencyEnricher. This annotation doesn't seem to be getting used
anywhere else.

Delete RemoveBuildAnnotationsEnricher & Move `maven.jkube.io/source-url`
annotation removal logic inside it to DependencyEnricher

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
@rohanKanojia rohanKanojia mentioned this issue Mar 30, 2022
Merged
12 tasks
@manusa manusa modified the milestones: 1.x, 1.8.0 Apr 21, 2022
rohanKanojia added a commit to rohanKanojia/jkube that referenced this issue Apr 21, 2022
@rohanKanojia
refactor (jkube-kit) : Remove RemoveBuildAnnotationsEnricher (eclipse…
c031f40 
…-jkube#1263)

RemoveBuildAnnotationsEnricher seems to be removing
`maven.jkube.io/source-url` annotation which gets added in
DependencyEnricher. This annotation doesn't seem to be getting used
anywhere else.

Delete RemoveBuildAnnotationsEnricher & Move `maven.jkube.io/source-url`
annotation removal logic inside it to DependencyEnricher

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
@rohanKanojia rohanKanojia mentioned this issue Apr 21, 2022
Closed
3 tasks
rohanKanojia added a commit to rohanKanojia/jkube that referenced this issue Apr 21, 2022
@rohanKanojia
refactor (jkube-kit) : Remove RemoveBuildAnnotationsEnricher (eclipse…
13e2991 
…-jkube#1263)

RemoveBuildAnnotationsEnricher seems to be removing
`maven.jkube.io/source-url` annotation which gets added in
DependencyEnricher. This annotation doesn't seem to be getting used
anywhere else.

Delete RemoveBuildAnnotationsEnricher & `maven.jkube.io/source-url`
annotation logic inside it to DependencyEnricher

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
rohanKanojia added a commit to rohanKanojia/jkube that referenced this issue Apr 21, 2022
@rohanKanojia
refactor (jkube-kit) : Remove RemoveBuildAnnotationsEnricher (eclipse…
6bb7fab 
…-jkube#1263)

RemoveBuildAnnotationsEnricher seems to be removing
`maven.jkube.io/source-url` annotation which gets added in
DependencyEnricher. This annotation doesn't seem to be getting used
anywhere else.

Delete RemoveBuildAnnotationsEnricher & `maven.jkube.io/source-url`
annotation logic inside it to DependencyEnricher

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
rohanKanojia added a commit to rohanKanojia/jkube that referenced this issue Apr 22, 2022
@rohanKanojia
refactor (jkube-kit) : Remove RemoveBuildAnnotationsEnricher (eclipse…
2ba9434 
…-jkube#1263)

RemoveBuildAnnotationsEnricher seems to be removing
`maven.jkube.io/source-url` annotation which gets added in
DependencyEnricher. This annotation doesn't seem to be getting used
anywhere else.

Delete RemoveBuildAnnotationsEnricher & `maven.jkube.io/source-url`
annotation logic inside it to DependencyEnricher

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
manusa pushed a commit that referenced this issue Apr 22, 2022
@rohanKanojia @manusa
refactor (jkube-kit) : Remove RemoveBuildAnnotationsEnricher (#1263)
48ab34b 
RemoveBuildAnnotationsEnricher seems to be removing
`maven.jkube.io/source-url` annotation which gets added in
DependencyEnricher. This annotation doesn't seem to be getting used
anywhere else.

Delete RemoveBuildAnnotationsEnricher & `maven.jkube.io/source-url`
annotation logic inside it to DependencyEnricher

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rohanKanojia rohanKanojia

Labels
technical-debt
Projects
None yet
Milestone
1.8.0
Development

Successfully merging a pull request may close this issue.

refactor (jkube-kit) : Remove RemoveBuildAnnotationsEnricher (#1263) rohanKanojia/jkube
3 participants
@manusa @rohanKanojia @Hrushi20