Propagate edgeworkload annotations #278
Conversation
jordigilh
force-pushed
the
propagate_workload_annotations
branch
3 times, most recently
from
551a3d7
to
c32a8fb
Compare
| @@ -302,7 +302,8 @@ func (a *ConfigurationAssembler) toWorkloadList(ctx context.Context, logger *zap | |||
| workload := models.Workload{ | |||
| Name: edgeworkload.Name, | |||
| Namespace: edgeworkload.Namespace, | |||
| Labels: labels.GetPodmanLabels(edgeworkload.Labels), | |||
| Annotations: utils.FilterByPodmanPrefix(edgeworkload.Annotations), | |||
| Labels: utils.FilterByPodmanPrefix(edgeworkload.Labels), | |||
There was a problem hiding this comment.
This will be good to have a check on the tests, so we make sure that at least one is in there.
There was a problem hiding this comment.
Added a test each for Labels and Annotations.
jordigilh
force-pushed
the
propagate_workload_annotations
branch
2 times, most recently
from
8b41622
to
71d1ab0
Compare
|
please commit all the files :) |
jordigilh
force-pushed
the
propagate_workload_annotations
branch
from
71d1ab0
to
02a0eb6
Compare
Like with Labels, annotations that start with podman/ will be propagated to the workload object that is sent to the agent. The agent will use these annotations in the pod manifest
jordigilh
force-pushed
the
propagate_workload_annotations
branch
from
02a0eb6
to
5491fa9
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jordigilh, masayag The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
| // And secondly, of those keys that have the "podman/" prefix, it uses the remaining part of the key as the new key in the returning map with the same value as in the original map | ||
| // e.g. 'podman/this-is-my-key:1' is stored in the returning map as 'this-is-my-key:1' | ||
| func FilterByPodmanPrefix(keyValues map[string]string) map[string]string { | ||
| ret := map[string]string{} |
There was a problem hiding this comment.
why not make(map[string]string) ? just a question
There was a problem hiding this comment.
Personal choice, I only use make when I need to create a map, channel or slice with a predefined size. Otherwise I keep it consistent with {}.
There was a problem hiding this comment.
ok but here you have a size. maximum what you can get is len(keyValues)
There was a problem hiding this comment.
Indeed, I just didn't consider that it would normally grow to be up to that size, so I chose to minimize mem allocation over cpu cycles, but I'm fine creating it with the given maximum size. WDYT?
There was a problem hiding this comment.
I pprofed 2 code one with/without make(map[string]string)
without make this is the mem:
1762.94kB 40.77% 40.77% 1762.94kB 40.77% runtime/pprof.StartCPUProfile
512.50kB 11.85% 52.63% 512.50kB 11.85% runtime.allocm
512.20kB 11.85% 64.47% 512.20kB 11.85% runtime.malg
512.05kB 11.84% 76.32% 512.05kB 11.84% regexp/syntax.(*parser).newRegexp (inline)
512.04kB 11.84% 88.16% 512.04kB 11.84% runtime.bgscavenge
512.02kB 11.84% 100% 512.02kB 11.84% runtime.gcBgMarkWorker
0 0% 100% 512.05kB 11.84% internal/profile.init
0 0% 100% 1762.94kB 40.77% main.main
0 0% 100% 512.05kB 11.84% regexp.Compile (inline)
0 0% 100% 512.05kB 11.84% regexp.MustCompile
with make:
1184.27kB 36.63% 36.63% 1184.27kB 36.63% runtime/pprof.StartCPUProfile
1024.41kB 31.68% 68.31% 1024.41kB 31.68% runtime.malg
512.50kB 15.85% 84.16% 512.50kB 15.85% runtime.allocm
512.02kB 15.84% 100% 512.02kB 15.84% runtime.gcBgMarkWorker
0 0% 100% 1184.27kB 36.63% main.main
0 0% 100% 1184.27kB 36.63% runtime.main
0 0% 100% 512.50kB 15.85% runtime.mstart
0 0% 100% 512.50kB 15.85% runtime.mstart0
0 0% 100% 512.50kB 15.85% runtime.mstart1
0 0% 100% 512.50kB 15.85% runtime.newm
If you look at main in the case with make you allocated less memory. I think is related to how maps are layout in memory.
There was a problem hiding this comment.
No worries, I can raise a new one for this, but thanks a lot for taking the time to check this out! 😄
There was a problem hiding this comment.
this was purely theoretical. don't worry
|
/lgtm |
…ad_annotations Propagate edgeworkload annotations
Adds the possibility to propagate
annotationsin the edgeworkload to the pod manifest. This feature is needed to enable the use ofcrunannotationrun.oci.keep_original_groups: "1"which instructscrunto keep the same group Ids in the host user to the container user, enabling the possibility of accessing host devices inside the container based on group affinity by the host user.