feat (jkube-kit) : Initial draft for JKube Actions Summary (#1033) #1686
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 #1686 (2023-03-01T05:25:50Z) ⚙️ JKube E2E Tests (4300228797)
|
rohanKanojia
force-pushed
the
pr/issue1033
branch
2 times, most recently
from
5fc47fa
to
da09ca9
Compare
|
Kudos, SonarCloud Quality Gate passed! |
Codecov Report
@@ Coverage Diff @@
## master #1686 +/- ##
============================================
+ Coverage 55.77% 57.02% +1.25%
- Complexity 4287 4470 +183
============================================
Files 481 486 +5
Lines 21212 21751 +539
Branches 2841 2868 +27
============================================
+ Hits 11831 12404 +573
+ Misses 8173 8109 -64
- Partials 1208 1238 +30
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
Nice work,
|
2 tasks
rohanKanojia
force-pushed
the
pr/issue1033
branch
from
da09ca9
to
c0a6cd3
Compare
rohanKanojia
force-pushed
the
pr/issue1033
branch
14 times, most recently
from
efc8c66
to
78c26b6
Compare
rohanKanojia
force-pushed
the
pr/issue1033
branch
3 times, most recently
from
bfe7405
to
7e5c93a
Compare
rohanKanojia
force-pushed
the
pr/issue1033
branch
4 times, most recently
from
406b037
to
1d38e36
Compare
rohanKanojia
force-pushed
the
pr/issue1033
branch
from
1d38e36
to
eeb263f
Compare
|
@rohanKanojia when you have some times, would it be possible to rebase this PR ? |
rohanKanojia
force-pushed
the
pr/issue1033
branch
2 times, most recently
from
404ffbf
to
6f4a574
Compare
|
Kudos, SonarCloud Quality Gate passed! |
|
@rohanKanojia |
|
also when I run Are generators used in k8s:resource ? I am not sure, but if not, it should not be displayed right ? |
|
I checked and looks like generators are used in ResourceMojo to initialize which uses generators to initialize: Which then gets used in initializing JKubeEnricherContext: |
|
OK but doesn't make sense right ? |
|
@sunix : What goal did you run for this output? I tried running |
rohanKanojia
force-pushed
the
pr/issue1033
branch
2 times, most recently
from
8fd6782
to
e88f040
Compare
rohanKanojia
force-pushed
the
pr/issue1033
branch
from
e88f040
to
332a9b4
Compare
sunix
reviewed
Feb 16, 2023
| [[summary]] | ||
| = Summary | ||
|
|
||
| {plugin} can print a brief summary of the task it performed in order to give better idea of what's going on to the user. At the moment, most of the build related actions (build, push, resource, apply etc.) print summary. It can be disabled with `jkube.summaryEnabled` property. |
There was a problem hiding this comment.
Suggested change
| {plugin} can print a brief summary of the task it performed in order to give better idea of what's going on to the user. At the moment, most of the build related actions (build, push, resource, apply etc.) print summary. It can be disabled with `jkube.summaryEnabled` property. | |
| {plugin} prints a brief summary of the task it performed in order to give better idea of what's going on to the user. At the moment, most of the build related actions (build, push, resource, apply etc.) print summary. It can be disabled with `jkube.summaryEnabled` property. |
sunix
reviewed
Feb 16, 2023
|
|
||
| private void printApplySummary(Summary summary) { | ||
| if (StringUtils.isNotBlank(summary.getAppliedClusterUrl())) { | ||
| logger.info("Applied resources from %s", summary.getAppliedClusterUrl()); |
There was a problem hiding this comment.
Suggested change
| logger.info("Applied resources from %s", summary.getAppliedClusterUrl()); | |
| logger.info("Applied resources to %s", summary.getAppliedClusterUrl()); |
sunix
reviewed
Feb 16, 2023
| private void printKubernetesResourceSummary(List<KubernetesResourceSummary> kubernetesResourceSummaries) { | ||
| if (kubernetesResourceSummaries != null && !kubernetesResourceSummaries.isEmpty()) { | ||
| for (KubernetesResourceSummary kubernetesResourceSummary : kubernetesResourceSummaries) { | ||
| logger.info(LIST_ELEMENT, kubernetesResourceSummary.getResourceName()); |
There was a problem hiding this comment.
This is what I have
[INFO] k8s: Applied resources from https://192.168.49.2:8443/
[INFO] k8s: - quarkus
[INFO] k8s: * v1 Service
[INFO] k8s: * Namespace: default
[INFO] k8s: - quarkus
[INFO] k8s: * apps/v1 Deployment
[INFO] k8s: * Namespace: default
I would try to be minimalist here and only display kind/name:
[INFO] k8s: Applied resources from https://192.168.49.2:8443/: [ Service/quarkus, Deployment/quarkus ]
What do you think ?
There was a problem hiding this comment.
okay, I've modified it as per your suggestion. Where should be specify deployed namespace?
There was a problem hiding this comment.
I am unsure: are we always applying resources in the same namespace?
sunix
reviewed
Feb 16, 2023
| private void printResourceSummary(Summary summary, File baseDir) { | ||
| if (summary.getGeneratedResourceFiles() != null && !summary.getGeneratedResourceFiles().isEmpty()) { | ||
| if (summary.getEnrichersApplied() != null && summary.getEnrichersApplied().size() < 20) { | ||
| logger.info("Enrichers applied: [%s]", String.join(",", summary.getEnrichersApplied())); |
There was a problem hiding this comment.
Minor:
At the moment I have
[INFO] k8s: Enrichers applied: [jkube-dependency,jkube-controller,jkube-controller-from-configuration,jkube-service,jkube-image,jkube-portname,jkube-project-label,jkube-git,jkube-serviceaccount,jkube-prometheus,jkube-revision-history,jkube-name,jkube-metadata,jkube-pod-annotations,jkube-container-env-java-options]
Maybe it would be better with spaces
[INFO] k8s: Enrichers applied: [ jkube-dependency, jkube-controller, jkube-controller-from-configuration, jkube-service, jkube-image, jkube-portname, jkube-project-label, jkube-git, jkube-serviceaccount, jkube-prometheus, jkube-revision-history, jkube-name, jkube-metadata, jkube-pod-annotations, jkube-container-env-java-options ]
332a9b4
to
b23dcfe
Compare
rohanKanojia
force-pushed
the
pr/issue1033
branch
from
b23dcfe
to
c1b4095
Compare
…kube#1033) Add summary for jkube build actions in order to improve ux. Signed-off-by: Rohan Kumar <rohaan@redhat.com>
Signed-off-by: Rohan Kumar <rohaan@redhat.com>
rohanKanojia
force-pushed
the
pr/issue1033
branch
from
c1b4095
to
6a5284e
Compare
|
Kudos, SonarCloud Quality Gate passed! |
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
Fixes #1033
Add summary for jkube build actions in order to improve ux. Summary is printed for
build,push,resource,helm,helm-push,apply, andundeployactions.Added a
SummaryandSummaryServiceclass which creates a temporary file storing state of Summary object as control goes from different parts of codebase. Both in case of maven and gradle, it's being checked whether current goal/task is last executing task and then only summary is being printed.Right now basic details are printed after small jkube banner. This output can be changed as per requirements:
Signed-off-by: Rohan Kumar rohaan@redhat.com
Type of change
test, version modification, documentation, etc.)
Checklist