Skip to content

feat: print x/y progress to the stdout #4545

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

Conversation

ricardomaraschini
Copy link
Contributor

@ricardomaraschini ricardomaraschini commented Apr 11, 2024

What this PR does / why we need it:

print a counter to show progress while pushing images and embedded cluster artifacts to the registry. we want this output so we can show a better feedback to the user when installing a kots application on an embedded cluster instance.

output sample:

root@ec:~#  /var/lib/embedded-cluster/bin/kubectl-kots \
        install \
        embedded-cluster-smoke-test-staging-app/ci-airgap \
        --license-file ./license.yaml \
        --namespace kotsadm \
        --app-version-label appver-dev-7976976 \
        --exclude-admin-console \
        --airgap-bundle ./embedded-cluster-smoke-test-staging-app.airgap
  • Validating registry information ✓
  • Deploying application
Pushing image 1/1
Pushing image 10.103.35.61:5000/embedded-cluster-smoke-test-staging-app/nginx:latest
Copying blob 035788421403 skipped: already exists
Copying blob c5cdd1ce752d skipped: already exists
Copying blob 8a1e25ce7c4f skipped: already exists
Copying blob 87c3fb37cbf2 skipped: already exists
Copying blob 39fc875bd2b2 skipped: already exists
Copying blob e78b137be355 skipped: already exists
Copying blob 33952c599532 skipped: already exists
Copying config 92b11f6764 done   |
Writing manifest to image destination
Pushing embedded cluster artifact 1/4
Pushing embedded cluster artifact 2/4
Pushing embedded cluster artifact 3/4
Pushing embedded cluster artifact 4/4
  • Waiting for Admin Console to be ready ✓
  • Uploading app archive
  • Waiting for installation to complete ✗
Error: failed to validate installation: license already exists for app embedded-cluster-smoke-test-staging-app
root@ec:~#

Which issue(s) this PR fixes:

Relates #102373

Does this PR introduce a user-facing change?

Add more feedback to the user while pushing Images and Embedded Cluster artifacts.

@ricardomaraschini ricardomaraschini added the type::feature New feature or request label Apr 11, 2024
@ricardomaraschini ricardomaraschini force-pushed the ricardomaraschini/sc-102373/add-better-output-to-explain-that-images branch from affbbda to f6d5abd Compare April 11, 2024 08:25
print a counter to show progress while pushing images and embedded
cluster artifacts.
@ricardomaraschini ricardomaraschini force-pushed the ricardomaraschini/sc-102373/add-better-output-to-explain-that-images branch from f6d5abd to 04d66ef Compare April 11, 2024 09:12
@ricardomaraschini ricardomaraschini marked this pull request as ready for review April 11, 2024 09:12
laverya
laverya previously approved these changes Apr 11, 2024
@chris-sanders
Copy link
Member

chris-sanders commented Apr 11, 2024

Proxying a comment from slack. Can we just show "Pushing Image 1/1" and update that line until all of the images are pushed, and then move on to showing "Pushing Embedded Cluster Artifact 1/4". The rest can be filtered, and of course it would be nice if we could put it back in the bullet point like the other lines.

Basically treat each of the "Pushing" lines the way we do the 0/2 for Admin console.

Copy link
Member

@ajp-io ajp-io left a comment

Choose a reason for hiding this comment

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

We can disregard Chris's comment proxied from Slack. I looked at the terminal output in the PR description, which was not up to date. I like what is in the code, so this is good.

@ricardomaraschini ricardomaraschini merged commit 1013d8e into main Apr 11, 2024
105 checks passed
@ricardomaraschini ricardomaraschini deleted the ricardomaraschini/sc-102373/add-better-output-to-explain-that-images branch April 11, 2024 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants