Skip to content

Commit

Permalink
endpoint: set labels outside NewEndpointFromChange
Browse files Browse the repository at this point in the history
When endpoint API is used to create an endpoint and `EndpointChangeRequest`
contains labels, it will not allocate an identity to the endpoint.

During `NewEndpointFromChangeModel()` labels are stored in the endpoint
model, causing the followup call `ep.UpdateLabels()` to not bump revision
during this single `createEndpoint()` call. This means the folloup call to
`e.runIdentityResolver()` never happens and the endpoint ends up without
identity and with state waiting-for-identity.

```
ENDPOINT   IDENTITY        LABELS (source:key[=value])               IPv4         STATUS

3236       <no label id>   k8s:app=incubator-mynetns3                10.247.1.1   waiting-for-identity
                           k8s:io.cilium.k8s.policy.cluster=default
                           k8s:io.kubernetes.pod.namespace=default
```

This should be allowed, otherwise user can only not set the labels
during `createEndpoint()` call and do a followup call `patchEndpoint()` where
labels will be set which then triggers regeneration to be triggered and
identity allocated.

This commit fixes the above issue by letting the caller handle the setting
of the endpoint labels and remove them from `NewEndpointFromChangeModel()`.

```
ENDPOINT   IDENTITY   LABELS (source:key[=value])                IPv4         STATUS

331        21864      k8s:app=incubator-mynetns3                 10.247.1.1   ready
                      k8s:io.cilium.k8s.policy.cluster=default
                      k8s:io.kubernetes.pod.namespace=default
```

Fixes: #29776

Signed-off-by: Ondrej Blazek <ondrej.blazek@firma.seznam.cz>
  • Loading branch information
oblazek committed Jan 10, 2024
1 parent a823186 commit 7481149
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 8 deletions.
7 changes: 7 additions & 0 deletions daemon/cmd/endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -627,6 +627,13 @@ func patchEndpointIDHandler(d *Daemon, params PatchEndpointIDParams) middleware.
return api.Error(PutEndpointIDInvalidCode, err2)
}

if epTemplate.Labels != nil {
lbls := labels.NewLabelsFromModel(epTemplate.Labels)
identityLabels, infoLabels := labelsfilter.Filter(lbls)
newEp.OpLabels.OrchestrationIdentity = identityLabels
newEp.OpLabels.OrchestrationInfo = infoLabels
}

var validStateTransition bool

// Log invalid state transitions, but do not error out for backwards
Expand Down
8 changes: 0 additions & 8 deletions pkg/endpoint/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
identitymodel "github.com/cilium/cilium/pkg/identity/model"
"github.com/cilium/cilium/pkg/labels"
"github.com/cilium/cilium/pkg/labels/model"
"github.com/cilium/cilium/pkg/labelsfilter"
"github.com/cilium/cilium/pkg/mac"
"github.com/cilium/cilium/pkg/option"
"github.com/cilium/cilium/pkg/policy"
Expand Down Expand Up @@ -129,13 +128,6 @@ func NewEndpointFromChangeModel(ctx context.Context, owner regeneration.Owner, p
}
}

if base.Labels != nil {
lbls := labels.NewLabelsFromModel(base.Labels)
identityLabels, infoLabels := labelsfilter.Filter(lbls)
ep.OpLabels.OrchestrationIdentity = identityLabels
ep.OpLabels.OrchestrationInfo = infoLabels
}

if base.State != nil {
ep.setState(State(*base.State), "Endpoint creation")
}
Expand Down

0 comments on commit 7481149

Please sign in to comment.