toolshed/abra
4
4
Fork 23
Code Issues 82 Pull Requests 12 Packages Releases 45 Activity

feat: abra app logs shows task errors #395

Merged
p4u1 merged 1 commits from (deleted):logs-task-error into main 2023-12-14 13:15:24 +00:00
Conversation 4 Commits 1 Files Changed 2
+102 -100
p4u1 commented 2023-12-12 16:55:32 +00:00
Owner
Copy Link
Copy Source

The log command now checks for the ready state in the task list. If it is not ready. It shows the task errors. This might look like this:

ERRO[0000] Service abra-test-recipe_default_app: State rejected: No such image: ngaaaax:1.21.0 
ERRO[0000] Service abra-test-recipe_default_app: State preparing:  
ERRO[0000] Service abra-test-recipe_default_app: State rejected: No such image: ngaaaax:1.21.0 
ERRO[0000] Service abra-test-recipe_default_app: State rejected: No such image: ngaaaax:1.21.0 
ERRO[0000] Service abra-test-recipe_default_app: State rejected: No such image: ngaaaax:1.21.0

Feel free to suggest a better error ouptput.

Closes coop-cloud/organising#518

The log command now checks for the ready state in the task list. If it is not ready. It shows the task errors. This might look like this: ``` ERRO[0000] Service abra-test-recipe_default_app: State rejected: No such image: ngaaaax:1.21.0 ERRO[0000] Service abra-test-recipe_default_app: State preparing: ERRO[0000] Service abra-test-recipe_default_app: State rejected: No such image: ngaaaax:1.21.0 ERRO[0000] Service abra-test-recipe_default_app: State rejected: No such image: ngaaaax:1.21.0 ERRO[0000] Service abra-test-recipe_default_app: State rejected: No such image: ngaaaax:1.21.0 ``` Feel free to suggest a better error ouptput. Closes https://git.coopcloud.tech/coop-cloud/organising/issues/518
p4u1 force-pushed logs-task-error from e40d99bfe2 to 98671a9f92 2023-12-12 17:01:12 +00:00 Compare
decentral1se reviewed 2023-12-12 21:06:17 +00:00
decentral1se left a comment
Owner
Copy Link
Copy Source

Amazin' 💯

Amazin' 💯
👍 1
cli/app/logs.go Outdated
@@ -147,0 +99,4 @@
filters.Add("name", service.Spec.Name)
tasks, err := cl.TaskList(context.Background(), types.TaskListOptions{Filters: f})
if err != nil {
logrus.Fatal(err)
decentral1se commented 2023-12-12 21:02:06 +00:00
Owner
Copy Link
Copy Source

Slightly confusing that this function does return err and also logrus.Fatal(err). Maybe this is handier?

Slightly confusing that this function does `return err` and also `logrus.Fatal(err)`. Maybe this is handier?
p4u1 commented 2023-12-13 08:26:52 +00:00
Author
Owner
Copy Link
Copy Source

Good catch! Changed it to return err. I did not change the logrus.Fatal(err) in the goroutine below because otherwise I would need to create an error channel.

Good catch! Changed it to `return err`. I did not change the `logrus.Fatal(err)` in the goroutine below because otherwise I would need to create an error channel.
👍 1
decentral1se marked this conversation as resolved
pkg/config/app.go Outdated
@@ -120,3 +110,4 @@
return filters, nil
}
func fmtFilter(appendMode bool, exact bool, stack string, service string) string {
decentral1se commented 2023-12-12 21:04:37 +00:00
Owner
Copy Link
Copy Source

Missing a docstring? Pretty cryptic function otherwise. Can be taken from the Filters docstring.

Missing a docstring? Pretty cryptic function otherwise. Can be taken from the `Filters` docstring.
p4u1 commented 2023-12-13 08:27:44 +00:00
Author
Owner
Copy Link
Copy Source

I added a docstring and simplified the function by moving the appendMode into the tailLogs function.

I added a docstring and simplified the function by moving the `appendMode` into the `tailLogs` function.
❤️ 1
decentral1se marked this conversation as resolved
pkg/config/app.go Outdated
@@ -603,3 +606,3 @@
// GetTimeoutFromLabel reads the timeout value from docker label "coop-cloud.${STACK_NAME}.TIMEOUT" and returns 50 as default value
func GetTimeoutFromLabel(compose *composetypes.Config, stackName string) (int, error) {
var timeout = 50 // Default Timeout
timeout := 50 // Default Timeout
decentral1se commented 2023-12-12 21:05:49 +00:00
Owner
Copy Link
Copy Source

Should we maybe abstract to pkg/config/env.go as a constant?

Should we maybe abstract to `pkg/config/env.go` as a constant?
p4u1 commented 2023-12-13 08:30:13 +00:00
Author
Owner
Copy Link
Copy Source

This was only changed because I formatted the whole file. I would prefer to make a change like this in a separate pr.

This was only changed because I formatted the whole file. I would prefer to make a change like this in a separate pr.
👍 1
decentral1se marked this conversation as resolved
p4u1 force-pushed logs-task-error from 98671a9f92 to 10e8bf0c7e 2023-12-13 08:24:36 +00:00 Compare
decentral1se approved these changes 2023-12-14 10:46:51 +00:00
decentral1se left a comment
Owner
Copy Link
Copy Source

LGTM! TYSM! 🤘

LGTM! TYSM! 🤘
p4u1 merged commit d4727db8f9 into main 2023-12-14 13:15:24 +00:00
p4u1 deleted branch logs-task-error 2023-12-14 13:15:25 +00:00
moritz commented 2023-12-21 00:25:20 +00:00
Member
Copy Link
Copy Source

Unfortunately this broke the secret handling completely. Most of the secrets can't be found anymore. Its because of the modified Filters function, that is used by the secrets.

Unfortunately this broke the secret handling completely. Most of the secrets can't be found anymore. Its because of the modified `Filters` function, that is used by the secrets.
p4u1 commented 2023-12-21 08:06:55 +00:00
Author
Owner
Copy Link
Copy Source

Very sorry about that! Can you be a bit more specific on how to reproduce the problem? Because for me it still works and also the integration tests for secrets still pass.

Very sorry about that! Can you be a bit more specific on how to reproduce the problem? Because for me it still works and also the integration tests for secrets still pass.
Sign in to join this conversation.
Reviewers
No Reviewers
decentral1se
Labels
Clear labels
bug
Something is not working
 build
go build related issues
 ci/cd
Building things with CI/CD
 critical fix
https://docs.coopcloud.tech/federation/resolutions/passed/010/
 design
UI/UX
 documentation
Documenting all the things
 duplicate
This issue or pull request already exists
 easy-first-issue
Something for new people to get stuck into. We hope it's easy!
 enhancement
New feature
 help wanted
Need some help
 i10n
Everything to do with localisation
 i18n
Everything to do with internationalisation
 installer
Everything to do with the install script.
 invalid
Something is wrong
 question
More information is needed
 release
Release management
 release-candidate
Related to the new release candidate
 security
Security related
 tech-debt
test
Unit/integration testing
 wontfix
This won't be fixed
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
3wordchant aadil (Aadil Ayub) abra-bot (Abra Bot) ammaratef45 amras (Sarma) Apfelwurm BornDeleuze Brooke carla cas (Cassowary) coopcloud cyrnel decentral1se (d1) dede devydave fauno (fauno) iexos jade (Jade Ambrose) jjsfunhouse jmakdah2 (Jackie Makdah) joe-irving (Joe Irving) kawaiipunk (KawaiiPunk) knoflook kolaente lambdabundesverband linnealovespie (April) moosemower moritz notplants oxaliq (sorrel) p4u1 pharaohgraphy (Andrew 🐦‍🔥❤️‍🔥✴️) renovate-bot (Comrade Renovate Bot) ripclap simon sixsmith (Sixsmith) stevensting trav (Trav Fryer) val (val (he/him)) yksflip
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: toolshed/abra#395