diff --git a/cmd/metal-api/internal/service/ip-service.go b/cmd/metal-api/internal/service/ip-service.go index cf7a504b1..a3484c1a0 100644 --- a/cmd/metal-api/internal/service/ip-service.go +++ b/cmd/metal-api/internal/service/ip-service.go @@ -21,6 +21,8 @@ import ( v1 "github.com/metal-stack/metal-api/cmd/metal-api/internal/service/v1" + goipam "github.com/metal-stack/go-ipam" + restfulspec "github.com/emicklei/go-restful-openapi/v2" restful "github.com/emicklei/go-restful/v3" "github.com/metal-stack/metal-lib/httperrors" @@ -90,18 +92,6 @@ func (r *ipResource) webService() *restful.WebService { Returns(http.StatusOK, "OK", []v1.IPResponse{}). DefaultReturns("Error", httperrors.HTTPErrorResponse{})) - ws.Route(ws.POST("/free/{id}"). - To(editor(r.freeIP)). - Operation("freeIPDeprecated"). - Doc("frees an ip"). - Param(ws.PathParameter("id", "identifier of the ip").DataType("string")). - Consumes(restful.MIME_JSON, "*/*"). - Metadata(restfulspec.KeyOpenAPITags, tags). - Writes(v1.IPResponse{}). - Returns(http.StatusOK, "OK", v1.IPResponse{}). - DefaultReturns("Error", httperrors.HTTPErrorResponse{}). - Deprecate()) - ws.Route(ws.DELETE("/free/{id}"). To(editor(r.freeIP)). Operation("freeIP"). @@ -320,13 +310,26 @@ func (r *ipResource) allocateIP(request *restful.Request, response *restful.Resp // TODO: Following operations should span a database transaction if possible - ipAddress, ipParentCidr, err := allocateIP(nw, specificIP, r.ipamer) - if err != nil { - r.sendError(request, response, defaultError(err)) - return + var ( + ipAddress string + ipParentCidr string + ) + + if specificIP == "" { + ipAddress, ipParentCidr, err = allocateRandomIP(nw, r.ipamer) + if err != nil { + r.sendError(request, response, defaultError(err)) + return + } + } else { + ipAddress, ipParentCidr, err = allocateSpecificIP(nw, specificIP, r.ipamer) + if err != nil { + r.sendError(request, response, defaultError(err)) + return + } } - r.logger(request).Debugw("found an ip to allocate", "ip", ipAddress, "network", nw.ID) + r.logger(request).Debugw("allocated ip in ipam", "ip", ipAddress, "network", nw.ID) ipType := metal.Ephemeral if requestPayload.Type == metal.Static { @@ -397,43 +400,48 @@ func (r *ipResource) updateIP(request *restful.Request, response *restful.Respon r.send(request, response, http.StatusOK, v1.NewIPResponse(&newIP)) } -func allocateIP(parent *metal.Network, specificIP string, ipamer ipam.IPAMer) (string, string, error) { - var ee []error - var err error - var ipAddress string - var parentPrefixCidr string +func allocateSpecificIP(parent *metal.Network, specificIP string, ipamer ipam.IPAMer) (ipAddress, parentPrefixCidr string, err error) { + parsedIP, err := netip.ParseAddr(specificIP) + if err != nil { + return "", "", fmt.Errorf("unable to parse specific ip: %w", err) + } + for _, prefix := range parent.Prefixes { - if specificIP == "" { - ipAddress, err = ipamer.AllocateIP(prefix) - } else { - pfx, perr := netip.ParsePrefix(prefix.String()) - if perr != nil { - return "", "", fmt.Errorf("unable to parse prefix %w", perr) - } - sip, serr := netip.ParseAddr(specificIP) - if serr != nil { - return "", "", fmt.Errorf("unable to parse specific ip %w", serr) - } - if !pfx.Contains(sip) { - continue - } - ipAddress, err = ipamer.AllocateSpecificIP(prefix, specificIP) - } + pfx, err := netip.ParsePrefix(prefix.String()) if err != nil { - ee = append(ee, err) + return "", "", fmt.Errorf("unable to parse prefix: %w", err) + } + + if !pfx.Contains(parsedIP) { continue } - if ipAddress != "" { - parentPrefixCidr = prefix.String() - break + + ipAddress, err = ipamer.AllocateSpecificIP(prefix, specificIP) + if err != nil && errors.Is(err, goipam.ErrAlreadyAllocated) { + return "", "", metal.Conflict("ip already allocated") + } + if err != nil { + return "", "", err } + + return ipAddress, prefix.String(), nil } - if ipAddress == "" { - if len(ee) > 0 { - return "", "", fmt.Errorf("cannot allocate free ip in ipam: %v", ee) + + return "", "", fmt.Errorf("specific ip not contained in any of the defined prefixes") +} + +func allocateRandomIP(parent *metal.Network, ipamer ipam.IPAMer) (ipAddress, parentPrefixCidr string, err error) { + for _, prefix := range parent.Prefixes { + ipAddress, err = ipamer.AllocateIP(prefix) + if err != nil && errors.Is(err, goipam.ErrNoIPAvailable) { + continue + } + if err != nil { + return "", "", err } - return "", "", errors.New("cannot allocate free ip in ipam") + + return ipAddress, prefix.String(), nil } - return ipAddress, parentPrefixCidr, nil + return "", "", metal.Internal("cannot allocate free ip in ipam, no ips left") } diff --git a/cmd/metal-api/internal/service/ip-service_test.go b/cmd/metal-api/internal/service/ip-service_test.go index 42f2d7ec7..cdef43305 100644 --- a/cmd/metal-api/internal/service/ip-service_test.go +++ b/cmd/metal-api/internal/service/ip-service_test.go @@ -145,7 +145,7 @@ func TestDeleteIP(t *testing.T) { for i := range tests { tt := tests[i] t.Run(tt.name, func(t *testing.T) { - req := httptest.NewRequest("POST", "/v1/ip/free/"+tt.ip, nil) + req := httptest.NewRequest("DELETE", "/v1/ip/free/"+tt.ip, nil) container = injectEditor(logger, container, req) req.Header.Add("Content-Type", "application/json") w := httptest.NewRecorder() @@ -244,7 +244,7 @@ func TestAllocateIP(t *testing.T) { }, specificIP: "11.0.0.5", wantedStatus: http.StatusUnprocessableEntity, - wantErr: errors.New("cannot allocate free ip in ipam"), + wantErr: errors.New("specific ip not contained in any of the defined prefixes"), }, } for i := range tests { diff --git a/cmd/metal-api/internal/service/machine-service.go b/cmd/metal-api/internal/service/machine-service.go index be1faa0e2..dce081ea5 100644 --- a/cmd/metal-api/internal/service/machine-service.go +++ b/cmd/metal-api/internal/service/machine-service.go @@ -1626,7 +1626,7 @@ func gatherUnderlayNetwork(ds *datastore.RethinkStore, partition *metal.Partitio func makeMachineNetwork(ds *datastore.RethinkStore, ipamer ipam.IPAMer, allocationSpec *machineAllocationSpec, n *allocationNetwork) (*metal.MachineNetwork, error) { if n.auto { - ipAddress, ipParentCidr, err := allocateIP(n.network, "", ipamer) + ipAddress, ipParentCidr, err := allocateRandomIP(n.network, ipamer) if err != nil { return nil, fmt.Errorf("unable to allocate an ip in network: %s %w", n.network.ID, err) } diff --git a/spec/metal-api.json b/spec/metal-api.json index 7182ad22f..c5ef503fa 100644 --- a/spec/metal-api.json +++ b/spec/metal-api.json @@ -6665,44 +6665,6 @@ "tags": [ "ip" ] - }, - "post": { - "consumes": [ - "*/*", - "application/json" - ], - "deprecated": true, - "operationId": "freeIPDeprecated", - "parameters": [ - { - "description": "identifier of the ip", - "in": "path", - "name": "id", - "required": true, - "type": "string" - } - ], - "produces": [ - "application/json" - ], - "responses": { - "200": { - "description": "OK", - "schema": { - "$ref": "#/definitions/v1.IPResponse" - } - }, - "default": { - "description": "Error", - "schema": { - "$ref": "#/definitions/httperrors.HTTPErrorResponse" - } - } - }, - "summary": "frees an ip", - "tags": [ - "ip" - ] } }, "/v1/ip/{id}": {