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

2024-11-30T06:59:03Z

ammaratef45 commented

2024-11-30 06:59:03 +00:00

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

continuous-integration/drone/push Build is passing

Details

continuous-integration/drone/pr Build is passing

Details

5a3c69d1f5

ammaratef45 referenced this pull request from toolshed/organising

2024-11-30 07:06:04 +00:00

SetRecipeLabel: panic: assignment to entry in nil map #643

decentral1se reviewed 2024-11-30 11:02:59 +00:00

decentral1se left a comment

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

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

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

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 referenced this pull request

2024-12-01 04:17:06 +00:00

[fix] chaos mode always fails deploy #450

ammaratef45 marked the pull request as work in progress 2024-12-01 04:20:29 +00:00

decentral1se commented

2024-12-28 21:25:37 +00:00

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