[flexible-ipam] Fix IP annotation not work on a StatefulSet #5715
Conversation
|
/test-flexible-ipam-e2e |
gran-vmv
force-pushed
the
ipam-stsip
branch
2 times, most recently
from
036f7bb
to
0395a1d
Compare
gran-vmv
force-pushed
the
ipam-stsip
branch
2 times, most recently
from
4c3cef1
to
8167259
Compare
| want bool | ||
| want1 []net.IP |
There was a problem hiding this comment.
| want bool | |
| want1 []net.IP | |
| hasIPPool bool | |
| expectedIPs []net.IP |
| assert.Equalf(t, want, got, "getIPPoolsForStatefulSet()") | ||
| assert.Equalf(t, tt.want1, got1, "getIPPoolsForStatefulSet()") |
There was a problem hiding this comment.
I feel it's better to replace "getIPPoolsForStatefulSet()" with some helpful messages.
There was a problem hiding this comment.
Changed to
assert.Equalf(t, want, got, "Unexpected IPPool result")
assert.Equalf(t, tt.expectedIPs, got1, "Unexpected IP result")
| splittedIPStrings := strings.Split(ipStrings, annotation.AntreaIPAMAnnotationDelimiter) | ||
| for _, ipString := range splittedIPStrings { | ||
| ip := net.ParseIP(ipString) | ||
| if ipString != "" && ip == nil { |
There was a problem hiding this comment.
The check is confusing. If ipString could be "", it will append nil to ips, I don't think we expect that?
| for _, ipString := range splittedIPStrings { | ||
| ip := net.ParseIP(ipString) | ||
| if ipString != "" && ip == nil { | ||
| klog.Errorf("invalid IP annotation %s", ipStrings) |
There was a problem hiding this comment.
- Use structured logging
- Captilize first letter
- Add more context, lack of information by just "invalid IP annotation"
Check https://github.com/tnqn/code-review-comments#logging for suggestions.
There was a problem hiding this comment.
Changed to klog.ErrorS(fmt.Errorf("invalid IP format"), "Ignore IP annotation", "PodIPs", ipStrings)
| for _, ipString := range splittedIPStrings { | ||
| ip := net.ParseIP(ipString) | ||
| if ip == nil { | ||
| klog.ErrorS(fmt.Errorf("invalid IP format"), "Ignore IP annotation", "PodIPs", ipStrings) |
There was a problem hiding this comment.
| klog.ErrorS(fmt.Errorf("invalid IP format"), "Ignore IP annotation", "PodIPs", ipStrings) | |
| klog.ErrorS(nil, "Ignored invalid Pod IP annotation in the StatefulSet template", "annotation", ipStrings, "statefulSet", klog.KObj(ss)) |
Signed-off-by: gran <gran@vmware.com>
|
/test-flexible-ipam-e2e |
|
/test-conformance |
2 similar comments
|
/test-conformance |
|
/test-conformance |
Make IP pre-allocation working on StatefulSet.