Browse code
libnetwork/drivers/macvlan, ipvlan: make driver.leave a stub
These drivers did not do anything meaningful in the `Leave` method; they
would check if the network and/or endpoint were missing, in which case
they produced an error, but the network and endpoint (if present) would
not be used, so it was only validation.
Such validation could still be relevant elsewhere, but looking at where
this method is called; the `Driver.Leave()` is called in two places, both
of which don't handle the error, other than logging it as a warning / error;
It's called by `Endpoint.sbJoin()`, as part of the rollback;
https://github.com/moby/moby/blob/d5c838dc5ee1a692c2fb6cde6da6fa185e7538aa/daemon/libnetwork/endpoint.go#L539-L545
And `Endpoint.sbLeave()`, which also discards the error;
https://github.com/moby/moby/blob/d5c838dc5ee1a692c2fb6cde6da6fa185e7538aa/daemon/libnetwork/endpoint.go#L772-L776
Based on he above, this code looks to be redundant, so replacing it with
a stub; returning `nil`.
As replacing the code removed the use of network.getEndpoint, which was effectively
a copy of network.endpoint (which didn't have error handling), I merged the two
methods, and removed custom error-handling elsewhere.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Sebastiaan van Stijn authored on 2025/10/19 03:26:36Showing 7 changed files
- daemon/libnetwork/drivers/ipvlan/ipvlan_endpoint.go
- daemon/libnetwork/drivers/ipvlan/ipvlan_joinleave.go
- daemon/libnetwork/drivers/ipvlan/ipvlan_state.go
- daemon/libnetwork/drivers/macvlan/macvlan_endpoint.go
- daemon/libnetwork/drivers/macvlan/macvlan_joinleave.go
- daemon/libnetwork/drivers/macvlan/macvlan_state.go
- daemon/libnetwork/endpoint.go
| ... | ... |
@@ -68,9 +68,9 @@ func (d *driver) DeleteEndpoint(nid, eid string) error {
|
| 68 | 68 |
if n == nil {
|
| 69 | 69 |
return fmt.Errorf("network id %q not found", nid)
|
| 70 | 70 |
} |
| 71 |
- ep := n.endpoint(eid) |
|
| 72 |
- if ep == nil {
|
|
| 73 |
- return fmt.Errorf("endpoint id %q not found", eid)
|
|
| 71 |
+ ep, err := n.endpoint(eid) |
|
| 72 |
+ if err != nil {
|
|
| 73 |
+ return err |
|
| 74 | 74 |
} |
| 75 | 75 |
if link, err := ns.NlHandle().LinkByName(ep.srcName); err == nil {
|
| 76 | 76 |
if err := ns.NlHandle().LinkDel(link); err != nil {
|
| ... | ... |
@@ -51,9 +51,9 @@ func (d *driver) Join(ctx context.Context, nid, eid string, sboxKey string, jinf |
| 51 | 51 |
if err != nil {
|
| 52 | 52 |
return err |
| 53 | 53 |
} |
| 54 |
- ep := n.endpoint(eid) |
|
| 55 |
- if ep == nil {
|
|
| 56 |
- return fmt.Errorf("could not find endpoint with id %s", eid)
|
|
| 54 |
+ ep, err := n.endpoint(eid) |
|
| 55 |
+ if err != nil {
|
|
| 56 |
+ return err |
|
| 57 | 57 |
} |
| 58 | 58 |
// bind the generated iface name to the endpoint |
| 59 | 59 |
// |
| ... | ... |
@@ -165,18 +165,6 @@ func (d *driver) Join(ctx context.Context, nid, eid string, sboxKey string, jinf |
| 165 | 165 |
|
| 166 | 166 |
// Leave method is invoked when a Sandbox detaches from an endpoint. |
| 167 | 167 |
func (d *driver) Leave(nid, eid string) error {
|
| 168 |
- network, err := d.getNetwork(nid) |
|
| 169 |
- if err != nil {
|
|
| 170 |
- return err |
|
| 171 |
- } |
|
| 172 |
- endpoint, err := network.getEndpoint(eid) |
|
| 173 |
- if err != nil {
|
|
| 174 |
- return err |
|
| 175 |
- } |
|
| 176 |
- if endpoint == nil {
|
|
| 177 |
- return fmt.Errorf("could not find endpoint with id %s", eid)
|
|
| 178 |
- } |
|
| 179 |
- |
|
| 180 | 168 |
return nil |
| 181 | 169 |
} |
| 182 | 170 |
|
| ... | ... |
@@ -5,7 +5,6 @@ package ipvlan |
| 5 | 5 |
import ( |
| 6 | 6 |
"context" |
| 7 | 7 |
"errors" |
| 8 |
- "fmt" |
|
| 9 | 8 |
|
| 10 | 9 |
"github.com/containerd/log" |
| 11 | 10 |
"github.com/moby/moby/v2/daemon/libnetwork/types" |
| ... | ... |
@@ -47,11 +46,18 @@ func (d *driver) getNetworks() []*network {
|
| 47 | 47 |
return ls |
| 48 | 48 |
} |
| 49 | 49 |
|
| 50 |
-func (n *network) endpoint(eid string) *endpoint {
|
|
| 50 |
+func (n *network) endpoint(eid string) (*endpoint, error) {
|
|
| 51 |
+ if eid == "" {
|
|
| 52 |
+ return nil, errors.New("invalid endpoint id")
|
|
| 53 |
+ } |
|
| 51 | 54 |
n.Lock() |
| 52 | 55 |
defer n.Unlock() |
| 53 | 56 |
|
| 54 |
- return n.endpoints[eid] |
|
| 57 |
+ ep, ok := n.endpoints[eid] |
|
| 58 |
+ if !ok || ep == nil {
|
|
| 59 |
+ return nil, errors.New("could not find endpoint with id " + eid)
|
|
| 60 |
+ } |
|
| 61 |
+ return ep, nil |
|
| 55 | 62 |
} |
| 56 | 63 |
|
| 57 | 64 |
func (n *network) addEndpoint(ep *endpoint) {
|
| ... | ... |
@@ -66,19 +72,6 @@ func (n *network) deleteEndpoint(eid string) {
|
| 66 | 66 |
n.Unlock() |
| 67 | 67 |
} |
| 68 | 68 |
|
| 69 |
-func (n *network) getEndpoint(eid string) (*endpoint, error) {
|
|
| 70 |
- n.Lock() |
|
| 71 |
- defer n.Unlock() |
|
| 72 |
- if eid == "" {
|
|
| 73 |
- return nil, fmt.Errorf("endpoint id %s not found", eid)
|
|
| 74 |
- } |
|
| 75 |
- if ep, ok := n.endpoints[eid]; ok {
|
|
| 76 |
- return ep, nil |
|
| 77 |
- } |
|
| 78 |
- |
|
| 79 |
- return nil, nil |
|
| 80 |
-} |
|
| 81 |
- |
|
| 82 | 69 |
func validateID(nid, eid string) error {
|
| 83 | 70 |
if nid == "" {
|
| 84 | 71 |
return errors.New("invalid network id")
|
| ... | ... |
@@ -72,9 +72,9 @@ func (d *driver) DeleteEndpoint(nid, eid string) error {
|
| 72 | 72 |
if n == nil {
|
| 73 | 73 |
return fmt.Errorf("network id %q not found", nid)
|
| 74 | 74 |
} |
| 75 |
- ep := n.endpoint(eid) |
|
| 76 |
- if ep == nil {
|
|
| 77 |
- return fmt.Errorf("endpoint id %q not found", eid)
|
|
| 75 |
+ ep, err := n.endpoint(eid) |
|
| 76 |
+ if err != nil {
|
|
| 77 |
+ return err |
|
| 78 | 78 |
} |
| 79 | 79 |
if link, err := ns.NlHandle().LinkByName(ep.srcName); err == nil {
|
| 80 | 80 |
if err := ns.NlHandle().LinkDel(link); err != nil {
|
| ... | ... |
@@ -39,9 +39,9 @@ func (d *driver) Join(ctx context.Context, nid, eid string, sboxKey string, jinf |
| 39 | 39 |
if err != nil {
|
| 40 | 40 |
return err |
| 41 | 41 |
} |
| 42 |
- ep := n.endpoint(eid) |
|
| 43 |
- if ep == nil {
|
|
| 44 |
- return fmt.Errorf("could not find endpoint with id %s", eid)
|
|
| 42 |
+ ep, err := n.endpoint(eid) |
|
| 43 |
+ if err != nil {
|
|
| 44 |
+ return err |
|
| 45 | 45 |
} |
| 46 | 46 |
// bind the generated iface name to the endpoint |
| 47 | 47 |
// |
| ... | ... |
@@ -124,18 +124,6 @@ func (d *driver) Join(ctx context.Context, nid, eid string, sboxKey string, jinf |
| 124 | 124 |
|
| 125 | 125 |
// Leave method is invoked when a Sandbox detaches from an endpoint. |
| 126 | 126 |
func (d *driver) Leave(nid, eid string) error {
|
| 127 |
- network, err := d.getNetwork(nid) |
|
| 128 |
- if err != nil {
|
|
| 129 |
- return err |
|
| 130 |
- } |
|
| 131 |
- endpoint, err := network.getEndpoint(eid) |
|
| 132 |
- if err != nil {
|
|
| 133 |
- return err |
|
| 134 |
- } |
|
| 135 |
- if endpoint == nil {
|
|
| 136 |
- return fmt.Errorf("could not find endpoint with id %s", eid)
|
|
| 137 |
- } |
|
| 138 |
- |
|
| 139 | 127 |
return nil |
| 140 | 128 |
} |
| 141 | 129 |
|
| ... | ... |
@@ -5,7 +5,6 @@ package macvlan |
| 5 | 5 |
import ( |
| 6 | 6 |
"context" |
| 7 | 7 |
"errors" |
| 8 |
- "fmt" |
|
| 9 | 8 |
|
| 10 | 9 |
"github.com/containerd/log" |
| 11 | 10 |
"github.com/moby/moby/v2/daemon/libnetwork/types" |
| ... | ... |
@@ -47,11 +46,18 @@ func (d *driver) getNetworks() []*network {
|
| 47 | 47 |
return ls |
| 48 | 48 |
} |
| 49 | 49 |
|
| 50 |
-func (n *network) endpoint(eid string) *endpoint {
|
|
| 50 |
+func (n *network) endpoint(eid string) (*endpoint, error) {
|
|
| 51 |
+ if eid == "" {
|
|
| 52 |
+ return nil, errors.New("invalid endpoint id")
|
|
| 53 |
+ } |
|
| 51 | 54 |
n.Lock() |
| 52 | 55 |
defer n.Unlock() |
| 53 | 56 |
|
| 54 |
- return n.endpoints[eid] |
|
| 57 |
+ ep, ok := n.endpoints[eid] |
|
| 58 |
+ if !ok || ep == nil {
|
|
| 59 |
+ return nil, errors.New("could not find endpoint with id " + eid)
|
|
| 60 |
+ } |
|
| 61 |
+ return ep, nil |
|
| 55 | 62 |
} |
| 56 | 63 |
|
| 57 | 64 |
func (n *network) addEndpoint(ep *endpoint) {
|
| ... | ... |
@@ -66,19 +72,6 @@ func (n *network) deleteEndpoint(eid string) {
|
| 66 | 66 |
n.Unlock() |
| 67 | 67 |
} |
| 68 | 68 |
|
| 69 |
-func (n *network) getEndpoint(eid string) (*endpoint, error) {
|
|
| 70 |
- n.Lock() |
|
| 71 |
- defer n.Unlock() |
|
| 72 |
- if eid == "" {
|
|
| 73 |
- return nil, fmt.Errorf("endpoint id %s not found", eid)
|
|
| 74 |
- } |
|
| 75 |
- if ep, ok := n.endpoints[eid]; ok {
|
|
| 76 |
- return ep, nil |
|
| 77 |
- } |
|
| 78 |
- |
|
| 79 |
- return nil, nil |
|
| 80 |
-} |
|
| 81 |
- |
|
| 82 | 69 |
func validateID(nid, eid string) error {
|
| 83 | 70 |
if nid == "" {
|
| 84 | 71 |
return errors.New("invalid network id")
|
| ... | ... |
@@ -538,8 +538,8 @@ func (ep *Endpoint) sbJoin(ctx context.Context, sb *Sandbox, options ...Endpoint |
| 538 | 538 |
} |
| 539 | 539 |
defer func() {
|
| 540 | 540 |
if retErr != nil {
|
| 541 |
- if e := d.Leave(nid, epid); e != nil {
|
|
| 542 |
- log.G(ctx).Warnf("driver leave failed while rolling back join: %v", e)
|
|
| 541 |
+ if err := d.Leave(nid, epid); err != nil {
|
|
| 542 |
+ log.G(ctx).WithError(err).Warnf("driver leave failed while rolling back join")
|
|
| 543 | 543 |
} |
| 544 | 544 |
} |
| 545 | 545 |
}() |