Fix typos in pkg/network #16793
Fix typos in pkg/network #16793
Conversation
openshift-merge-robot
added
the
approved
openshift-ci-robot
added
the
size/S
|
@openshift/networking PTAL |
| @@ -36,9 +37,9 @@ func ValidateClusterNetwork(clusterNet *networkapi.ClusterNetwork) field.ErrorLi | |||
| } else { | |||
| maskLen, addrLen := clusterIPNet.Mask.Size() | |||
| if clusterNet.HostSubnetLength > uint32(addrLen-maskLen) { | |||
| allErrs = append(allErrs, field.Invalid(field.NewPath("hostSubnetLength"), clusterNet.HostSubnetLength, "subnet length is too large for clusterNetwork")) | |||
There was a problem hiding this comment.
Are you sure this should not be camelCase?
There was a problem hiding this comment.
Yes (refer https://github.com/openshift/origin/blob/master/pkg/network/apis/network/v1/types.go)
Even though camel case is the general norm, we incorrectly published the field name as 'hostsubnetlength', so this error should refer to the same name.
Anyway this field under ClusterNetwork is deprecated and the new HostSubnetLength under ClusterNetworkEntry is using camel case notation.
pkg/network/master/master.go
Outdated
| @@ -94,7 +94,7 @@ func Start(networkConfig osconfigapi.MasterNetworkConfig, networkClient networkc | |||
| log.Infof("Created ClusterNetwork %s", common.ClusterNetworkToString(configCN)) | |||
|
|
|||
| if err = master.checkClusterNetworkAgainstClusterObjects(); err != nil { | |||
| log.Errorf("WARNING: cluster contains objects incompatible with new ClusterNetwork: %v", err) | |||
| log.Warningf("Cluster contains objects incompatible with new ClusterNetwork: %v", err) | |||
There was a problem hiding this comment.
I had made this a log.Error intentionally to make it more likely to get noticed in the logs. (You can remove the "WARNING" if you want, but I think this should stay as log.Error.)
pkg/network/master/master.go
Outdated
| @@ -105,7 +105,7 @@ func Start(networkConfig osconfigapi.MasterNetworkConfig, networkClient networkc | |||
| configCN.TypeMeta = existingCN.TypeMeta | |||
| configCN.ObjectMeta = existingCN.ObjectMeta | |||
| if err = master.checkClusterNetworkAgainstClusterObjects(); err != nil { | |||
| log.Errorf("ERROR: Attempting to modify cluster to excludes existing objects: %v", err) | |||
| log.Errorf("Attempting to modify cluster to excludes existing objects: %v", err) | |||
There was a problem hiding this comment.
"excludes" should be "exclude" there...
| @@ -148,7 +148,7 @@ func (np *networkPolicyPlugin) AddNetNamespace(netns *networkapi.NetNamespace) { | |||
| defer np.lock.Unlock() | |||
|
|
|||
| if _, exists := np.namespaces[netns.NetID]; exists { | |||
| glog.Warning("Got AddNetNamespace for already-existing namespace %s (%d)", netns.NetName, netns.NetID) | |||
| glog.Warningf("Got AddNetNamespace for already-existing namespace %s (%d)", netns.NetName, netns.NetID) | |||
There was a problem hiding this comment.
Isn't govet supposed to catch those?
...
OK, apparently, no, it looks for Logf, Errorf, and Fatalf, but not Warningf. We need to pass an argument. I'll file a bug about that.
pravisankar
force-pushed
the
misc-fixes
branch
from
3dcc35a
to
fe3314b
Compare
|
@danwinship updated, ptal |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: danwinship, pravisankar The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
LGTM |
|
Automatic merge from submit-queue (batch tested with PRs 15807, 16778, 16801, 16804, 16793). |
Automatic merge from submit-queue (batch tested with PRs 15807, 16778, 16801, 16804, 16793). Make govet check glog.Info/Warning calls The default set of names covered by govet's checking of Printf-like functions includes Log, Error, and Fatal, but not Info or Warning. (Noticed in #16793.)
No description provided.