Skip to content

Commit af0c1c6

Browse files
jamandtimebertt
jamand
authored and
timebertt
committed
feat: Add InternalIPs to CreateMachineResponse (gardener#322)
1 parent 156e5a0 commit af0c1c6

File tree

3 files changed

+151
-23
lines changed

3 files changed

+151
-23
lines changed

‎pkg/driver/driver.go‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"github.com/gardener/machine-controller-manager/pkg/util/provider/driver"
1313
"github.com/gardener/machine-controller-manager/pkg/util/provider/machinecodes/codes"
1414
"github.com/gardener/machine-controller-manager/pkg/util/provider/machinecodes/status"
15+
corev1 "k8s.io/api/core/v1"
1516
"k8s.io/klog/v2"
1617

1718
"github.com/gardener/machine-controller-manager-provider-openstack/pkg/apis/cloudprovider"
@@ -71,16 +72,30 @@ func (p *OpenstackDriver) CreateMachine(ctx context.Context, req *driver.CreateM
7172
return nil, status.Error(mapErrorToCode(err), fmt.Sprintf("failed to construct context for the request: %v", err))
7273
}
7374

74-
providerID, err := ex.CreateMachine(ctx, req.Machine.Name, req.Secret.Data[cloudprovider.UserData])
75+
server, err := ex.CreateMachine(ctx, req.Machine.Name, req.Secret.Data[cloudprovider.UserData])
7576
if err != nil {
7677
klog.Errorf("machine creation for machine %q failed with: %v", req.Machine.Name, err)
7778
return nil, status.Error(mapErrorToCode(err), err.Error())
7879
}
7980

80-
return &driver.CreateMachineResponse{
81-
ProviderID: providerID,
81+
response := driver.CreateMachineResponse{
82+
ProviderID: server.ProviderID,
8283
NodeName: req.Machine.Name,
83-
}, nil
84+
}
85+
86+
if len(server.InternalIPs) > 0 {
87+
addresses := make([]corev1.NodeAddress, 0, len(server.InternalIPs))
88+
89+
for _, ip := range server.InternalIPs {
90+
addresses = append(addresses, corev1.NodeAddress{
91+
Type: corev1.NodeInternalIP,
92+
Address: ip,
93+
})
94+
}
95+
response.Addresses = addresses
96+
}
97+
98+
return &response, nil
8499
}
85100

86101
// InitializeMachine handles VM initialization for openstack VM's. Currently, un-implemented.

‎pkg/driver/executor/executor.go‎

Lines changed: 68 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,13 @@ type Executor struct {
3232
Config *api.MachineProviderConfig
3333
}
3434

35+
// CreateMachineResult return a slice of internal IPs under which the machine
36+
// with the given ID is reachable.
37+
type CreateMachineResult struct {
38+
ProviderID string
39+
InternalIPs []string
40+
}
41+
3542
// NewExecutor returns a new instance of Executor.
3643
func NewExecutor(factory *client.Factory, config *api.MachineProviderConfig) (*Executor, error) {
3744
computeClient, err := factory.Compute(client.WithRegion(config.Spec.Region))
@@ -59,9 +66,41 @@ func NewExecutor(factory *client.Factory, config *api.MachineProviderConfig) (*E
5966
return ex, nil
6067
}
6168

69+
// getServerIPs assumes the server has exactly one network interface
70+
// and extracts its internal IP addresses.
71+
func getServerIPs(server *servers.Server) ([]string, error) {
72+
ips := make([]string, 0)
73+
74+
if len(server.Addresses) != 1 {
75+
return nil, fmt.Errorf("expected 1 network, but found %d", len(server.Addresses))
76+
}
77+
78+
// Format of the addresses field: https://docs.openstack.org/api-ref/compute/#list-servers-detailed.
79+
for _, networkAddresses := range server.Addresses {
80+
addrList, ok := networkAddresses.([]any)
81+
if !ok {
82+
return nil, fmt.Errorf("could not assert network addresses to slice")
83+
}
84+
85+
// Iterate through the addresses (may be IPv4, IPv6).
86+
for _, addrData := range addrList {
87+
addressMap, ok := addrData.(map[string]any)
88+
if !ok {
89+
continue
90+
}
91+
92+
if ipAddress, ok := addressMap["addr"].(string); ok {
93+
ips = append(ips, ipAddress)
94+
}
95+
}
96+
}
97+
98+
return ips, nil
99+
}
100+
62101
// CreateMachine creates a new OpenStack server instance and waits until it reports "ACTIVE".
63102
// If there is an error during the build process, or if the building phase timeouts, it will delete any artifacts created.
64-
func (ex *Executor) CreateMachine(ctx context.Context, machineName string, userData []byte) (string, error) {
103+
func (ex *Executor) CreateMachine(ctx context.Context, machineName string, userData []byte) (*CreateMachineResult, error) {
65104
var (
66105
server *servers.Server
67106
err error
@@ -79,30 +118,44 @@ func (ex *Executor) CreateMachine(ctx context.Context, machineName string, userD
79118
if err == nil {
80119
klog.Infof("found existing server [Name=%q, ID=%q]", machineName, server.ID)
81120
} else if !errors.Is(err, ErrNotFound) {
82-
return "", err
121+
return nil, err
83122
} else {
84123
// clean-up function when creation fails in an intermediate step
85124
serverNetworks, err := ex.resolveServerNetworks(ctx, machineName)
86125
if err != nil {
87-
return "", deleteOnFail(fmt.Errorf("failed to resolve server [Name=%q] networks: %w", machineName, err))
126+
return nil, deleteOnFail(fmt.Errorf("failed to resolve server [Name=%q] networks: %w", machineName, err))
88127
}
89128

90129
server, err = ex.deployServer(ctx, machineName, userData, serverNetworks)
91130
if err != nil {
92-
return "", deleteOnFail(fmt.Errorf("failed to deploy server [Name=%q]: %w", machineName, err))
131+
return nil, deleteOnFail(fmt.Errorf("failed to deploy server [Name=%q]: %w", machineName, err))
93132
}
94133
}
95134

96-
err = ex.waitForServerStatus(ctx, server.ID, []string{client.ServerStatusBuild}, []string{client.ServerStatusActive}, 1200)
135+
// The server information when status is ACTIVE has addresses field populated
136+
var activeServer *servers.Server
137+
activeServer, err = ex.waitForServerStatus(ctx,
138+
server.ID,
139+
[]string{client.ServerStatusBuild},
140+
[]string{client.ServerStatusActive}, 1200)
97141
if err != nil {
98-
return "", deleteOnFail(fmt.Errorf("error waiting for server [ID=%q] to reach target status: %w", server.ID, err))
142+
return nil, deleteOnFail(fmt.Errorf("error waiting for server [ID=%q] to reach target status: %w", server.ID, err))
99143
}
100144

101-
if err := ex.patchServerPortsForPodNetwork(ctx, server.ID); err != nil {
102-
return "", deleteOnFail(fmt.Errorf("failed to patch server [ID=%q] ports: %s", server.ID, err))
145+
if err := ex.patchServerPortsForPodNetwork(ctx, activeServer.ID); err != nil {
146+
return nil, deleteOnFail(fmt.Errorf("failed to patch server [ID=%q] ports: %s", server.ID, err))
103147
}
104148

105-
return encodeProviderID(ex.Config.Spec.Region, server.ID), nil
149+
var internalIPs []string
150+
internalIPs, err = getServerIPs(activeServer)
151+
if err != nil {
152+
klog.Infof("failed to extract internal IPs [ID=%q] ports: %s", activeServer.ID, err)
153+
}
154+
155+
return &CreateMachineResult{
156+
ProviderID: encodeProviderID(ex.Config.Spec.Region, activeServer.ID),
157+
InternalIPs: internalIPs,
158+
}, nil
106159
}
107160

108161
// resolveServerNetworks resolves the network configuration for the server.
@@ -156,10 +209,11 @@ func (ex *Executor) resolveServerNetworks(ctx context.Context, machineName strin
156209
return serverNetworks, nil
157210
}
158211

159-
// waitForServerStatus blocks until the server with the specified ID reaches one of the target status.
212+
// waitForServerStatus blocks until the server with the specified ID reaches one of the target status and returns the server after reaching this status.
160213
// waitForServerStatus will fail if an error occurs, the operation it timeouts after the specified time, or the server status is not in the pending list.
161-
func (ex *Executor) waitForServerStatus(ctx context.Context, serverID string, pending []string, target []string, secs int) error {
162-
return wait.PollUntilContextTimeout(
214+
func (ex *Executor) waitForServerStatus(ctx context.Context, serverID string, pending []string, target []string, secs int) (*servers.Server, error) {
215+
var server *servers.Server
216+
return server, wait.PollUntilContextTimeout(
163217
ctx,
164218
10*time.Second,
165219
time.Duration(secs)*time.Second,
@@ -175,6 +229,7 @@ func (ex *Executor) waitForServerStatus(ctx context.Context, serverID string, pe
175229

176230
klog.V(5).Infof("waiting for server [ID=%q] and current status %v, to reach status %v.", serverID, current.Status, target)
177231
if strSliceContains(target, current.Status) {
232+
server = current
178233
return true, nil
179234
}
180235

@@ -468,7 +523,7 @@ func (ex *Executor) DeleteMachine(ctx context.Context, machineName, providerID s
468523
return err
469524
}
470525

471-
if err = ex.waitForServerStatus(ctx, server.ID, nil, []string{client.ServerStatusDeleted}, 1200); err != nil {
526+
if _, err = ex.waitForServerStatus(ctx, server.ID, nil, []string{client.ServerStatusDeleted}, 1200); err != nil {
472527
return fmt.Errorf("error while waiting for server [ID=%q] to be deleted: %v", server.ID, err)
473528
}
474529
} else if !errors.Is(err, ErrNotFound) {

‎pkg/driver/executor/executor_test.go‎

Lines changed: 64 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,8 @@ var _ = Describe("Executor", func() {
7575
networkID = "networkID"
7676
portID = "portID"
7777
podCidr = "10.0.0.0/16"
78+
serverIPv4 = "10.250.0.5"
79+
serverIPv6 = "2000:db0::1"
7880
)
7981
BeforeEach(func() {
8082
cfg = &openstack.MachineProviderConfig{
@@ -112,6 +114,14 @@ var _ = Describe("Executor", func() {
112114
compute.EXPECT().GetServer(ctx, serverID).Return(&servers.Server{
113115
ID: serverID,
114116
Status: client.ServerStatusActive,
117+
Addresses: map[string]any{
118+
"private": []any{
119+
map[string]any{
120+
"addr": serverIPv4,
121+
"version": 4,
122+
},
123+
},
124+
},
115125
}, nil))
116126
network.EXPECT().ListPorts(ctx, &ports.ListOpts{
117127
DeviceID: serverID,
@@ -120,9 +130,10 @@ var _ = Describe("Executor", func() {
120130
AllowedAddressPairs: &[]ports.AddressPair{{IPAddress: podCidr}},
121131
}).Return(nil)
122132

123-
providerId, err := ex.CreateMachine(ctx, machineName, nil)
133+
server, err := ex.CreateMachine(ctx, machineName, nil)
124134
Expect(err).ToNot(HaveOccurred())
125-
Expect(providerId).To(Equal(encodeProviderID(region, serverID)))
135+
Expect(server.InternalIPs).To(HaveLen(1))
136+
Expect(server.InternalIPs[0]).To(Equal(serverIPv4))
126137
})
127138

128139
It("should succeed when spec contains subnet", func() {
@@ -152,9 +163,9 @@ var _ = Describe("Executor", func() {
152163
AllowedAddressPairs: &[]ports.AddressPair{{IPAddress: podCidr}},
153164
}).Return(nil)
154165

155-
providerId, err := ex.CreateMachine(ctx, machineName, nil)
166+
server, err := ex.CreateMachine(ctx, machineName, nil)
156167
Expect(err).ToNot(HaveOccurred())
157-
Expect(providerId).To(Equal(encodeProviderID(region, serverID)))
168+
Expect(server.ProviderID).To(Equal(encodeProviderID(region, serverID)))
158169
})
159170

160171
It("should succeed when spec contains rootDisksize", func() {
@@ -193,9 +204,9 @@ var _ = Describe("Executor", func() {
193204
AllowedAddressPairs: &[]ports.AddressPair{{IPAddress: podCidr}},
194205
}).Return(nil)
195206

196-
providerId, err := ex.CreateMachine(ctx, machineName, nil)
207+
server, err := ex.CreateMachine(ctx, machineName, nil)
197208
Expect(err).ToNot(HaveOccurred())
198-
Expect(providerId).To(Equal(encodeProviderID(region, serverID)))
209+
Expect(server.ProviderID).To(Equal(encodeProviderID(region, serverID)))
199210
})
200211

201212
It("should delete the server on failure", func() {
@@ -229,6 +240,53 @@ var _ = Describe("Executor", func() {
229240
_, err := ex.CreateMachine(ctx, machineName, nil)
230241
Expect(err).To(HaveOccurred())
231242
})
243+
244+
It("should accept multiple internal IPs", func() {
245+
ex := &Executor{
246+
Compute: compute,
247+
Network: network,
248+
Config: cfg,
249+
}
250+
251+
compute.EXPECT().ListServers(ctx, &servers.ListOpts{Name: machineName}).Return([]servers.Server{}, nil)
252+
compute.EXPECT().ImageIDFromName(ctx, imageName).Return(images.Image{ID: "imageID"}, nil)
253+
compute.EXPECT().FlavorIDFromName(ctx, flavorName).Return("flavorID", nil)
254+
compute.EXPECT().CreateServer(ctx, gomock.Any(), gomock.Any()).Return(&servers.Server{
255+
ID: serverID,
256+
}, nil)
257+
gomock.InOrder(
258+
compute.EXPECT().GetServer(ctx, serverID).Return(&servers.Server{
259+
ID: serverID,
260+
Status: client.ServerStatusBuild,
261+
}, nil),
262+
compute.EXPECT().GetServer(ctx, serverID).Return(&servers.Server{
263+
ID: serverID,
264+
Status: client.ServerStatusActive,
265+
Addresses: map[string]any{
266+
"private": []any{
267+
map[string]any{
268+
"addr": serverIPv4,
269+
"version": 4,
270+
},
271+
map[string]any{
272+
"addr": serverIPv6,
273+
"version": 6,
274+
},
275+
},
276+
},
277+
}, nil))
278+
network.EXPECT().ListPorts(ctx, &ports.ListOpts{
279+
DeviceID: serverID,
280+
}).Return([]ports.Port{{NetworkID: networkID, ID: portID}}, nil)
281+
network.EXPECT().UpdatePort(ctx, portID, ports.UpdateOpts{
282+
AllowedAddressPairs: &[]ports.AddressPair{{IPAddress: podCidr}},
283+
}).Return(nil)
284+
285+
server, err := ex.CreateMachine(ctx, machineName, nil)
286+
Expect(err).ToNot(HaveOccurred())
287+
Expect(server.InternalIPs).To(HaveLen(2))
288+
Expect(server.InternalIPs).To(ConsistOf(serverIPv4, serverIPv6))
289+
})
232290
})
233291

234292
Context("List", func() {

0 commit comments

Comments
 (0)