toolshed/abra
toolshed/abra
4
2
Fork 18
Code Issues 27 Pull Requests 3 Packages Releases 40 Activity

WIP: check if labels are nil before adding labels #449

Closed
ammaratef45 wants to merge 1 commits from nil_labels into main
pull from: nil_labels
merge into: toolshed:main
Conversation 3 Commits 1 Files Changed 1 +11 -4
ammaratef45 commented 2024-11-30 06:59:03 +00:00
Member
Copy Link

For toolshed/organising#643

I tested it by playing with a local recipe, create an app instance without labels (failed to deploy initially), tried with the changes, deploy succeeded AND labels existed when I inspected the service on the swarm box 🎊

For https://git.coopcloud.tech/toolshed/organising/issues/643 I tested it by playing with a local recipe, create an app instance without labels (failed to deploy initially), tried with the changes, deploy succeeded AND labels existed when I inspected the service on the swarm box 🎊
ammaratef45 added 1 commit 2024-11-30 06:59:04 +00:00
check if labels are nil before adding labels
All checks were successful
continuous-integration/drone/push Build is passing
Details
continuous-integration/drone/pr Build is passing
Details
5a3c69d1f5
decentral1se reviewed 2024-11-30 11:02:59 +00:00
decentral1se left a comment
Owner
Copy Link

One minor comment, see what you think about it! Otherwise, very legit, tysm! 🤘

One minor comment, see what you think about it! Otherwise, very legit, tysm! 🤘
pkg/app/compose.go
@ -89,0 +89,4 @@
func AddLabel(service composetypes.ServiceConfig, labelKey string, value string) {
if service.Deploy.Labels == nil {
service.Deploy.Labels = composetypes.Labels{}
decentral1se commented 2024-11-30 11:01:35 +00:00
Owner
Copy Link

We have an unwritten (🙈) convention to use ...version=unknown when we don't know what the version label will be. For example:

bba1640913/pkg/upstream/stack/stack.go (L113)

More examples here.

So, maybe it's an idea to manually set the same config to be "unknown"?

deploy:
  labels:
    - "coop-cloud.${STACK_NAME}.version=unknown"

I think this might be important in the case of "set label" logic since other logic will try to read this label later on.

Or, maybe it's too complicated / too much assumption. I am not sure. This will set the label on the deployed containers but it won't be present on the compose config?

We have an unwritten (🙈) convention to use `...version=unknown` when we don't know what the version label will be. For example: https://git.coopcloud.tech/coop-cloud/abra/src/commit/bba1640913d4fe1f8b04d9ca606d9f967a9352b4/pkg/upstream/stack/stack.go#L113 More examples [here](https://git.coopcloud.tech/coop-cloud/abra/search?q=%22unknown%22). So, maybe it's an idea to manually set the same config to be "unknown"? ```yaml deploy: labels: - "coop-cloud.${STACK_NAME}.version=unknown" ``` I think this might be important in the case of "set label" logic since other logic will try to read this label later on. Or, maybe it's too complicated / too much assumption. I am not sure. This will set the label on the deployed containers but it won't be present on the compose config?
ammaratef45 commented 2024-12-01 04:20:15 +00:00
Author
Member
Copy Link

Thanks for this comment, it made me realize my change silences the error but doesn't really add the labels

Gonna dig deeper

Thanks for this comment, it made me realize my change silences the error but doesn't really add the labels Gonna dig deeper
👍 1
decentral1se marked this conversation as resolved
ammaratef45 commented 2024-11-30 23:39:51 +00:00
Author
Member
Copy Link

On a second thought, this solution didn't really work

I did docker service inspect <app_name>_app and saw Labels and was like Wohoo all good

The labels that exist are com.docker.stack.image and com.docker.stack.namespace 😥

On a second thought, this solution didn't really work I did `docker service inspect <app_name>_app` and saw `Labels` and was like Wohoo all good The labels that exist are `com.docker.stack.image` and `com.docker.stack.namespace` 😥
👍 1
ammaratef45 changed title from check if labels are nil before adding labels to WIP: check if labels are nil before adding labels 2024-12-01 04:20:29 +00:00
decentral1se commented 2024-12-28 21:25:37 +00:00
Owner
Copy Link

Took a pass on this one!

toolshed/organising#643 (comment)

Took a pass on this one! https://git.coopcloud.tech/toolshed/organising/issues/643#issuecomment-22046
decentral1se closed this pull request 2024-12-28 21:25:37 +00:00
All checks were successful
continuous-integration/drone/push Build is passing
Details
continuous-integration/drone/pr Build is passing
Details

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
bug
Something is not working
build
go build related issues
ci/cd
Building things with CI/CD
contributing
Dealing with contributing
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
enhancement
New feature
help wanted
Need some help
invalid
Something is wrong
meta
relating to the project or repo itself
question
More information is needed
release
Release management
release-candidate
Related to the new release candidate
security
Security related
test
Unit/integration testing
wontfix
This won't be fixed
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
3wordchant abra-bot ammaratef45 Apfelwurm carla cas decentral1se fauno kawaiipunk knoflook lambdabundesverband moritz p4u1 simon trav val yksflip
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: toolshed/abra#449
No description provided.
Delete Branch "nil_labels"

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?