Skip to content

Commit d185d21

Browse files
jamandjamand
authored
feat: Add InternalIPs to CreateMachineResponse (gardener#322)
1 parent 7a439d9 commit d185d21

3 files changed

Lines changed: 149 additions & 23 deletions

File tree

‎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: 66 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,11 @@ type Executor struct {
3232
Config *api.MachineProviderConfig
3333
}
3434

35+
type CreateMachineResult struct {
36+
ProviderID string
37+
InternalIPs []string
38+
}
39+
3540
// NewExecutor returns a new instance of Executor.
3641
func NewExecutor(factory *client.Factory, config *api.MachineProviderConfig) (*Executor, error) {
3742
computeClient, err := factory.Compute(client.WithRegion(config.Spec.Region))
@@ -59,9 +64,41 @@ func NewExecutor(factory *client.Factory, config *api.MachineProviderConfig) (*E
5964
return ex, nil
6065
}
6166

67+
// getServerIPs assumes the server has exactly one network interface
68+
// and extracts its internal IP addresses.
69+
func getServerIPs(server *servers.Server) ([]string, error) {
70+
ips := make([]string, 0)
71+
72+
if len(server.Addresses) != 1 {
73+
return nil, fmt.Errorf("expected 1 network, but found %d", len(server.Addresses))
74+
}
75+
76+
// Format of the addresses field: https://docs.openstack.org/api-ref/compute/#list-servers-detailed.
77+
for _, networkAddresses := range server.Addresses {
78+
addrList, ok := networkAddresses.([]any)
79+
if !ok {
80+
return nil, fmt.Errorf("could not assert network addresses to slice")
81+
}
82+
83+
// Iterate through the addresses (may be IPv4, IPv6).
84+
for _, addrData := range addrList {
85+
addressMap, ok := addrData.(map[string]any)
86+
if !ok {
87+
continue
88+
}
89+
90+
if ipAddress, ok := addressMap["addr"].(string); ok {
91+
ips = append(ips, ipAddress)
92+
}
93+
}
94+
}
95+
96+
return ips, nil
97+
}
98+
6299
// CreateMachine creates a new OpenStack server instance and waits until it reports "ACTIVE".
63100
// 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) {
101+
func (ex *Executor) CreateMachine(ctx context.Context, machineName string, userData []byte) (*CreateMachineResult, error) {
65102
var (
66103
server *servers.Server
67104
err error
@@ -79,30 +116,44 @@ func (ex *Executor) CreateMachine(ctx context.Context, machineName string, userD
79116
if err == nil {
80117
klog.Infof("found existing server [Name=%q, ID=%q]", machineName, server.ID)
81118
} else if !errors.Is(err, ErrNotFound) {
82-
return "", err
119+
return nil, err
83120
} else {
84121
// clean-up function when creation fails in an intermediate step
85122
serverNetworks, err := ex.resolveServerNetworks(ctx, machineName)
86123
if err != nil {
87-
return "", deleteOnFail(fmt.Errorf("failed to resolve server [Name=%q] networks: %w", machineName, err))
124+
return nil, deleteOnFail(fmt.Errorf("failed to resolve server [Name=%q] networks: %w", machineName, err))
88125
}
89126

90127
server, err = ex.deployServer(ctx, machineName, userData, serverNetworks)
91128
if err != nil {
92-
return "", deleteOnFail(fmt.Errorf("failed to deploy server [Name=%q]: %w", machineName, err))
129+
return nil, deleteOnFail(fmt.Errorf("failed to deploy server [Name=%q]: %w", machineName, err))
93130
}
94131
}
95132

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

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))
143+
if err := ex.patchServerPortsForPodNetwork(ctx, activeServer.ID); err != nil {
144+
return nil, deleteOnFail(fmt.Errorf("failed to patch server [ID=%q] ports: %s", server.ID, err))
103145
}
104146

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

108159
// resolveServerNetworks resolves the network configuration for the server.
@@ -156,10 +207,11 @@ func (ex *Executor) resolveServerNetworks(ctx context.Context, machineName strin
156207
return serverNetworks, nil
157208
}
158209

159-
// waitForServerStatus blocks until the server with the specified ID reaches one of the target status.
210+
// waitForServerStatus blocks until the server with the specified ID reaches one of the target status and returns the server after reaching this status.
160211
// 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(
212+
func (ex *Executor) waitForServerStatus(ctx context.Context, serverID string, pending []string, target []string, secs int) (*servers.Server, error) {
213+
var server *servers.Server
214+
return server, wait.PollUntilContextTimeout(
163215
ctx,
164216
10*time.Second,
165217
time.Duration(secs)*time.Second,
@@ -175,6 +227,7 @@ func (ex *Executor) waitForServerStatus(ctx context.Context, serverID string, pe
175227

176228
klog.V(5).Infof("waiting for server [ID=%q] and current status %v, to reach status %v.", serverID, current.Status, target)
177229
if strSliceContains(target, current.Status) {
230+
server = current
178231
return true, nil
179232
}
180233

@@ -468,7 +521,7 @@ func (ex *Executor) DeleteMachine(ctx context.Context, machineName, providerID s
468521
return err
469522
}
470523

471-
if err = ex.waitForServerStatus(ctx, server.ID, nil, []string{client.ServerStatusDeleted}, 1200); err != nil {
524+
if _, err = ex.waitForServerStatus(ctx, server.ID, nil, []string{client.ServerStatusDeleted}, 1200); err != nil {
472525
return fmt.Errorf("error while waiting for server [ID=%q] to be deleted: %v", server.ID, err)
473526
}
474527
} 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)