Skip to content

Commit 076ec19

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #470 from andrewsykim/fix-ip-addr
Do not error on multiple network devices if at least one has a valid IP address
2 parents dbcf8fd + 4244149 commit 076ec19

File tree

4 files changed

+95
-29
lines changed

4 files changed

+95
-29
lines changed

‎pkg/cloud/vsphere/context/cluster_context.go‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -309,13 +309,14 @@ func (c *ClusterContext) ControlPlaneEndpoint() (string, error) {
309309
return "", errors.Wrap(err, "error creating machine context while searching for control plane endpoint")
310310
}
311311

312-
if ipAddr := machineCtx.IPAddr(); ipAddr != "" {
313-
controlPlaneEndpoint := net.JoinHostPort(ipAddr, strconv.Itoa(int(machineCtx.BindPort())))
314-
machineCtx.Logger.V(2).Info("got control plane endpoint from machine", "control-plane-endpoint", controlPlaneEndpoint)
315-
return controlPlaneEndpoint, nil
312+
ipAddr, err := machineCtx.IPAddr()
313+
if err != nil {
314+
return "", errors.Wrap(err, "error getting first IP address for machine")
316315
}
317316

318-
return "", errors.New("unable to get control plane endpoint")
317+
controlPlaneEndpoint := net.JoinHostPort(ipAddr, strconv.Itoa(int(machineCtx.BindPort())))
318+
machineCtx.Logger.V(2).Info("got control plane endpoint from machine", "control-plane-endpoint", controlPlaneEndpoint)
319+
return controlPlaneEndpoint, nil
319320
}
320321

321322
// Patch updates the object and its status on the API server.

‎pkg/cloud/vsphere/context/machine_context.go‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@ import (
3838
"sigs.k8s.io/cluster-api-provider-vsphere/pkg/record"
3939
)
4040

41+
// ErrNoMachineIPAddr indicates that no valid IP addresses were found in a machine context
42+
var ErrNoMachineIPAddr = errors.New("no IP addresses found for machine")
43+
4144
// MachineContextParams are the parameters needed to create a MachineContext.
4245
type MachineContextParams struct {
4346
ClusterContextParams
@@ -170,18 +173,17 @@ func (c *MachineContext) HasControlPlaneRole() bool {
170173
}
171174

172175
// IPAddr returns the machine's first IP address.
173-
func (c *MachineContext) IPAddr() string {
176+
func (c *MachineContext) IPAddr() (string, error) {
174177
if c.Machine == nil {
175-
return ""
178+
return "", errors.New("machine in context is nil")
176179
}
177180

178181
var err error
179182
var preferredAPIServerCIDR *net.IPNet
180183
if c.MachineConfig.Network.PreferredAPIServerCIDR != "" {
181184
_, preferredAPIServerCIDR, err = net.ParseCIDR(c.MachineConfig.Network.PreferredAPIServerCIDR)
182185
if err != nil {
183-
c.Logger.Error(err, "error parsing preferred apiserver CIDR")
184-
return ""
186+
return "", errors.New("error parsing preferred apiserver CIDR")
185187
}
186188

187189
c.Logger.V(4).Info("detected preferred apiserver CIDR", "preferredAPIServerCIDR", preferredAPIServerCIDR)
@@ -193,15 +195,15 @@ func (c *MachineContext) IPAddr() string {
193195
}
194196

195197
if preferredAPIServerCIDR == nil {
196-
return nodeAddr.Address
198+
return nodeAddr.Address, nil
197199
}
198200

199201
if preferredAPIServerCIDR.Contains(net.ParseIP(nodeAddr.Address)) {
200-
return nodeAddr.Address
202+
return nodeAddr.Address, nil
201203
}
202204
}
203205

204-
return ""
206+
return "", ErrNoMachineIPAddr
205207
}
206208

207209
// BindPort returns the machine's API bind port.
@@ -232,10 +234,15 @@ func (c *MachineContext) ControlPlaneEndpoint() (string, error) {
232234
return controlPlaneEndpoint, nil
233235
}
234236

235-
ipAddr := c.IPAddr()
236-
if ipAddr == "" || !c.HasControlPlaneRole() {
237-
return "", errors.New("unable to get control plane endpoint")
237+
if !c.HasControlPlaneRole() {
238+
return "", errors.New("machine is not a control plane node")
238239
}
240+
241+
ipAddr, err := c.IPAddr()
242+
if err != nil {
243+
return "", errors.Wrapf(err, "failed to get first IP address for machine %q", c)
244+
}
245+
239246
controlPlaneEndpoint := net.JoinHostPort(ipAddr, strconv.Itoa(int(c.BindPort())))
240247
c.Logger.V(2).Info("got control plane endpoint from machine", "control-plane-endpoint", controlPlaneEndpoint)
241248
return controlPlaneEndpoint, nil

‎pkg/cloud/vsphere/context/machine_context_test.go‎

Lines changed: 66 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,10 @@ import (
2828

2929
func Test_MachineContextIPAddr(t *testing.T) {
3030
tests := []struct {
31-
name string
32-
ctx *MachineContext
33-
ipAddr string
31+
name string
32+
ctx *MachineContext
33+
ipAddr string
34+
expectedErr error
3435
}{
3536
{
3637
name: "single IPv4 address, no preferred CIDR",
@@ -49,7 +50,8 @@ func Test_MachineContextIPAddr(t *testing.T) {
4950
},
5051
},
5152
},
52-
ipAddr: "192.168.0.1",
53+
ipAddr: "192.168.0.1",
54+
expectedErr: nil,
5355
},
5456
{
5557
name: "single IPv6 address, no preferred CIDR",
@@ -68,7 +70,8 @@ func Test_MachineContextIPAddr(t *testing.T) {
6870
},
6971
},
7072
},
71-
ipAddr: "fdf3:35b5:9dad:6e09::0001",
73+
ipAddr: "fdf3:35b5:9dad:6e09::0001",
74+
expectedErr: nil,
7275
},
7376
{
7477
name: "multiple IPv4 addresses, only 1 internal, no preferred CIDR",
@@ -95,7 +98,8 @@ func Test_MachineContextIPAddr(t *testing.T) {
9598
},
9699
},
97100
},
98-
ipAddr: "192.168.0.1",
101+
ipAddr: "192.168.0.1",
102+
expectedErr: nil,
99103
},
100104
{
101105
name: "multiple IPv4 addresses, preferred CIDR set to v4",
@@ -123,7 +127,8 @@ func Test_MachineContextIPAddr(t *testing.T) {
123127
},
124128
},
125129
},
126-
ipAddr: "192.168.0.1",
130+
ipAddr: "192.168.0.1",
131+
expectedErr: nil,
127132
},
128133
{
129134
name: "multiple IPv4 and IPv6 addresses, preferred CIDR set to v4",
@@ -151,7 +156,8 @@ func Test_MachineContextIPAddr(t *testing.T) {
151156
},
152157
},
153158
},
154-
ipAddr: "192.168.0.1",
159+
ipAddr: "192.168.0.1",
160+
expectedErr: nil,
155161
},
156162
{
157163
name: "multiple IPv4 and IPv6 addresses, preferred CIDR set to v6",
@@ -179,13 +185,63 @@ func Test_MachineContextIPAddr(t *testing.T) {
179185
},
180186
},
181187
},
182-
ipAddr: "fdf3:35b5:9dad:6e09::0001",
188+
ipAddr: "fdf3:35b5:9dad:6e09::0001",
189+
expectedErr: nil,
190+
},
191+
{
192+
name: "no addresses found",
193+
ctx: &MachineContext{
194+
ClusterContext: &ClusterContext{
195+
Logger: klogr.New(),
196+
},
197+
MachineConfig: &v1alpha1.VsphereMachineProviderSpec{
198+
Network: v1alpha1.NetworkSpec{},
199+
},
200+
Machine: &clusterv1.Machine{
201+
Status: clusterv1.MachineStatus{
202+
Addresses: []corev1.NodeAddress{},
203+
},
204+
},
205+
},
206+
ipAddr: "",
207+
expectedErr: ErrNoMachineIPAddr,
208+
},
209+
{
210+
name: "no addresses found with preferred CIDR",
211+
ctx: &MachineContext{
212+
ClusterContext: &ClusterContext{
213+
Logger: klogr.New(),
214+
},
215+
MachineConfig: &v1alpha1.VsphereMachineProviderSpec{
216+
Network: v1alpha1.NetworkSpec{
217+
PreferredAPIServerCIDR: "192.168.0.0/16",
218+
},
219+
},
220+
Machine: &clusterv1.Machine{
221+
Status: clusterv1.MachineStatus{
222+
Addresses: []corev1.NodeAddress{
223+
{
224+
Type: corev1.NodeInternalIP,
225+
Address: "10.0.0.1",
226+
},
227+
},
228+
},
229+
},
230+
},
231+
ipAddr: "",
232+
expectedErr: ErrNoMachineIPAddr,
183233
},
184234
}
185235

186236
for _, test := range tests {
187237
t.Run(test.name, func(t *testing.T) {
188-
ipAddr := test.ctx.IPAddr()
238+
ipAddr, err := test.ctx.IPAddr()
239+
if err != test.expectedErr {
240+
t.Logf("expected err: %q", test.expectedErr)
241+
t.Logf("actual err: %q", err)
242+
t.Errorf("unexpected error")
243+
}
244+
189245
if ipAddr != test.ipAddr {
190246
t.Logf("expected IP addr: %q", test.ipAddr)
191247
t.Logf("actual IP addr: %q", ipAddr)

‎pkg/cloud/vsphere/services/govmomi/update.go‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -154,17 +154,19 @@ func reconcileNetwork(ctx *context.MachineContext, vm *object.VirtualMachine) er
154154
}
155155
if powerState == types.VirtualMachinePowerStatePoweredOn {
156156
for _, netStatus := range ctx.MachineStatus.Network {
157-
if len(netStatus.IPAddrs) == 0 {
158-
ctx.Logger.V(6).Info("reenqueue to wait on IP addresses")
159-
return &clustererror.RequeueAfterError{RequeueAfter: config.DefaultRequeue}
160-
}
161157
for _, ip := range netStatus.IPAddrs {
162158
ipAddrs = append(ipAddrs, corev1.NodeAddress{
163159
Type: corev1.NodeInternalIP,
164160
Address: ip,
165161
})
166162
}
167163
}
164+
165+
if len(ipAddrs) == 0 {
166+
ctx.Logger.V(6).Info("requeuing to wait on IP addresses")
167+
return &clustererror.RequeueAfterError{RequeueAfter: config.DefaultRequeue}
168+
}
169+
168170
}
169171

170172
// Use the collected IP addresses to assign the Machine's addresses.

0 commit comments

Comments
 (0)