abra 0.10 app deploy output is less informative #550
No Assignees
3wordchant
3wordchant
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: toolshed/abra#550
Reference in New Issue
Block a user
No description provided.
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
previously
abra app deploylisted the secrets and configs it was creating or reusing, but now it only shows progress of container deployment. i found the previous output useful to quickly check if the correct version of things was being deployed, specially when doing chaotic stuff.unsure now if i should get used to it or claim for it to come back :P
coop-cloud/hedgedoc#17 (comment)
There is some discussion already to add this stuff back. What do you think about only showing this information when it changed? See #537 (comment)
showing it when it changes sounds good!
Yeh apologies, I'm kinda freestyling the deploy output and it's still not great IMHO.
All those messages from before are now moved to
DEBUGlogging messages, so can easily be brought back after "initialising deploy" or whatever it says now. The only issue with "when it changed" is that it only shows when it changes on <v0.10.x? E.g. create/update. So, do we mean that we ignore an "update" (you already know it was created?).Merging these two in here:
Some stuff is for "before deploy" (overview?) and others for "post deploy" (but we have UI for that yet besides logs).
Related: #556
An attempt at UI design
Original:
Secrets
Configs
(showing incipient upgrade if possible, otherwise from post-deploy log messages)
Images
But: for long image tags like
php:7.4-fpm-alpine3.13this could make the display Very Wide. Should we break it onto separate lines for upgrades, rather? Or is it rare enough that this is an OK default?i like how it shows what's changing!
for logs, i always go to tap since it shows ok / not ok and it's easy to read. maybe something like that, with custom prefixes? what abra version had the INFO/WARN/etc prefixes? i'm not sure i've seen them lately, but i've only been deploying upgrades
I think current
mainabra (still?) has those prefixes for abra's own logs:For Docker logs (to display after "INFO initialising deployment", I'm not aware of the Docker daemon giving us log-level info; if there is a way of getting it then 💯 it would be great to add those same prefix badges.
Anyway, two ❤️ on the proposed UI I will aim to implement it this week.
@3wordchant check out
pkg/ui/deploy.gofor the gory details! we're receiving all messages from the runtime and decoding and choosing / throwing away some to show on theabraside. i've tried to make more sense of that but it's pretty chaotic... throw in a debug statement somewhere and you'll get a taste for it! Loving the UI developments here 🙏Could drop the image and just show the new tag to save some chars, e.g.
Learning from #611, it seems like you'll need to do hook into when the code path is doing the checking out of specific versions and collect all the compose configs and build up your bookkeeping until the deploy overview.
Do really like the proposed changes here, I think it'll make for a great improvement.
Progress so far:
The main remaining thing is to show proposed image changes.
I am running into some weird inconsistencies between config names, e.g. Nextcloud has a config called
nginx_confbut the actual Docker secret name is${STACK_NAME}_nginx_${NGINX_CONF_VERSION}(i.e. see missing_conf).I guess this points to needing to do more intelligent parsing of compose files to match the remote config names 🤔
https://git.coopcloud.tech/coop-cloud/nextcloud/src/branch/main/compose.yml#L155-L158
Latest:
Looking OK y'all? @decentral1se @fauno @p4u1
I'll write some tests and do some code cleanup meanwhile.
Also I have come to think this is just a recipe packaging mistake which we don't need to support.
If the Docker config is called
nginx_confthen both the long name (foo_example_com_nginx_conf_v1) and the version variable (${NGINX_CONF_VERSION}) should match.Adding parsing of the un-interpolated
configs.*.nameto get at the variable name directly seems too wacky.Hell yes, that overview is looking incredible! Great work 👏 Totally convinced by the need for now #639 🙃 I guess this is the information we were holding in our heads all along when running deployment operations and now we don't have to! It's a very informative overview now.
looks great! but inevitable feedback (!)
could you make cells be vertically aligned to top? when the second column is multiline, it makes you think the content belongs to the previous row
empty line between rows is more readable to me!
are you taking into account the current tty's columns? i'm worried tables too wide will break and people won't know it's because their tty's are too narrow.
somewhere we discussed the difference between domain and url, maybe add the url separately? so it's also clickable on terminal's that support autolinking.
I really like the new ui!! One thing though is that it is quite verbose. I would love an option where it only shows what changes and omits anything that's unchanged. Maybe we could even make that the default. The verbose option could be shown with --debug or something
Thanks for the feedback!
Yeh maybe
--debugor a new flag (--full/-Fidk 🤷) for everything and otherwise, only showing what was changed? I'd be curious if the "only changed" output also makes sense. I agree it is quite verbose. If we also show the logs from the deployment, it's gonna be a lot of text.#639
I believe it is flexible according to the width / resize. @3wordchant can you confirm? (I can help with this if not.)
Thanks for all the feedback folks! 🙏
It seems to fall apart 😬
The string causing the most problems in my earlier example is
docker.elastic.co/elasticsearch/elasticsearch: 8.17.2 (new), maybe stripping the registry is the move?I would suggest
--show-unchanged/-U;-Fseems confusing similar to--force/-fto me, and--debugis going to be a LOT of output.#643 - imo it's possible this might need some separate discussion.
Ah right yeh. OK, so we output this table and then we prompt for input. The code isn't responding to resizes of the window at that stage and can't handle it. I don't think there is much we can do about that tbh. What we can control is that the output of the (re)deploy overview fits the original terminal size? I am sorry to report that it doesn't look like we're doing that in
DeployOverview🙈.... we do handle it elsewhere:Maybe fixing this can be for another PR?
I think the suggestion from @p4u1 was to make the default "only changed" (less verbose) and "--$flag" do "show everything" (more verbose)? I am not sure where I stand on it cus I'd need to play with it but fine to go with whatever you both find is workable! Also just getting it out and seeing what people think about it is also good!
Sorry for more noise - there is a way around it: we would instead of using
body.WriteString()inCreateOverview, initiate a fulltea.NewProgram(...)inDeployOverview(like we do inWaitOnServices) for showing both the overview and showing the prompt. That would mean we'd be in full control of the output and could respond totea.WindowSizeMsg(like inpkg/ui/deploy.go). Yes, this would be best in another ticket 😅 Sorry for the ones with small terminals!Agreed.
So proposal is:
abra app deploy foo.bar.com→ only shows changed images & configsabra app deploy foo.bar.com --show-unchanged/abra app deploy foo.bar.com -U→ shows all images and configs