Merge pull request #13124 from enj/enj/i/sdn_name_func/12697,12959

Merged by openshift-bot

Author: OpenShift Bot

Diff for: api/protobuf-spec/github_com_openshift_origin_pkg_sdn_api_v1.proto

@@ -26657,7 +26657,7 @@
      },
      "host": {
       "type": "string",
-      "description": "Host is the name of the node. (This is redundant with the object's name, and this field is not actually used any more.)"
+      "description": "Host is the name of the node. (This is the same as the object's name, but both fields must be set.)"
      },
      "hostIP": {
       "type": "string",

Diff for: api/swagger-spec/oapi-v1.json

@@ -50864,7 +50864,7 @@
       "type": "string"
      },
      "host": {
-      "description": "Host is the name of the node. (This is redundant with the object's name, and this field is not actually used any more.)",
+      "description": "Host is the name of the node. (This is the same as the object's name, but both fields must be set.)",
       "type": "string"
      },
      "hostIP": {

Diff for: api/swagger-spec/openshift-openapi-spec.json

@@ -13228,7 +13228,7 @@ var OpenAPIDefinitions *common.OpenAPIDefinitions = &common.OpenAPIDefinitions{
 					},
 					"host": {
 						SchemaProps: spec.SchemaProps{
-							Description: "Host is the name of the node. (This is redundant with the object's name, and this field is not actually used any more.)",
+							Description: "Host is the name of the node. (This is the same as the object's name, but both fields must be set.)",
 							Type:        []string{"string"},
 							Format:      "",
 						},

Diff for: pkg/openapi/zz_generated.openapi.go

@@ -79,7 +79,7 @@ func (EgressNetworkPolicySpec) SwaggerDoc() map[string]string {
 var map_HostSubnet = map[string]string{
 	"":         "HostSubnet describes the container subnet network on a node. The HostSubnet object must have the same name as the Node object it corresponds to.",
 	"metadata": "Standard object's metadata.",
-	"host":     "Host is the name of the node. (This is redundant with the object's name, and this field is not actually used any more.)",
+	"host":     "Host is the name of the node. (This is the same as the object's name, but both fields must be set.)",
 	"hostIP":   "HostIP is the IP address to be used as a VTEP by other nodes in the overlay network",
 	"subnet":   "Subnet is the CIDR range of the overlay network assigned to the node for its pods",
 }

Diff for: pkg/sdn/api/v1/generated.proto

@@ -45,8 +45,7 @@ type HostSubnet struct {
 	// Standard object's metadata.
 	kapi.ObjectMeta `json:"metadata,omitempty" protobuf:"bytes,1,opt,name=metadata"`
 
-	// Host is the name of the node. (This is redundant with the object's name, and this
-	// field is not actually used any more.)
+	// Host is the name of the node. (This is the same as the object's name, but both fields must be set.)
 	Host string `json:"host" protobuf:"bytes,2,opt,name=host"`
 	// HostIP is the IP address to be used as a VTEP by other nodes in the overlay network
 	HostIP string `json:"hostIP" protobuf:"bytes,3,opt,name=hostIP"`

Diff for: pkg/sdn/api/v1/swagger_doc.go

@@ -1,6 +1,7 @@
 package validation
 
 import (
+	"fmt"
 	"net"
 
 	"k8s.io/kubernetes/pkg/api/validation"
@@ -85,6 +86,10 @@ func ValidateClusterNetworkUpdate(obj *sdnapi.ClusterNetwork, old *sdnapi.Cluste
 func ValidateHostSubnet(hs *sdnapi.HostSubnet) field.ErrorList {
 	allErrs := validation.ValidateObjectMeta(&hs.ObjectMeta, false, path.ValidatePathSegmentName, field.NewPath("metadata"))
 
+	if hs.Host != hs.Name {
+		allErrs = append(allErrs, field.Invalid(field.NewPath("host"), hs.Host, fmt.Sprintf("must be the same as metadata.name: %q", hs.Name)))
+	}
+
 	if hs.Subnet == "" {
 		// check if annotation exists, then let the Subnet field be empty
 		if _, ok := hs.Annotations[sdnapi.AssignHostSubnetAnnotation]; !ok {
@@ -117,8 +122,12 @@ func ValidateHostSubnetUpdate(obj *sdnapi.HostSubnet, old *sdnapi.HostSubnet) fi
 func ValidateNetNamespace(netnamespace *sdnapi.NetNamespace) field.ErrorList {
 	allErrs := validation.ValidateObjectMeta(&netnamespace.ObjectMeta, false, path.ValidatePathSegmentName, field.NewPath("metadata"))
 
+	if netnamespace.NetName != netnamespace.Name {
+		allErrs = append(allErrs, field.Invalid(field.NewPath("netname"), netnamespace.NetName, fmt.Sprintf("must be the same as metadata.name: %q", netnamespace.Name)))
+	}
+
 	if err := sdnapi.ValidVNID(netnamespace.NetID); err != nil {
-		allErrs = append(allErrs, field.Invalid(field.NewPath("netID"), netnamespace.NetID, err.Error()))
+		allErrs = append(allErrs, field.Invalid(field.NewPath("netid"), netnamespace.NetID, err.Error()))
 	}
 	return allErrs
 }

Diff for: pkg/sdn/api/v1/types.go

@@ -11,7 +11,6 @@ import (
 	"time"
 
 	kapi "k8s.io/kubernetes/pkg/api"
-	kubeerr "k8s.io/kubernetes/pkg/api/errors"
 	"k8s.io/kubernetes/pkg/api/meta"
 	"k8s.io/kubernetes/pkg/api/unversioned"
 	"k8s.io/kubernetes/pkg/client/restclient"
@@ -154,12 +153,12 @@ var etcdStorageData = map[unversioned.GroupVersionResource]struct {
 	// --
 
 	// github.com/openshift/origin/pkg/sdn/api/v1
-	gvr("", "v1", "netnamespaces"): { // This will fail to delete because meta.name != NetName but it is keyed off NetName
-		stub:             `{"metadata": {"name": "nn1"}, "netid": 100, "netname": "networkname"}`,
+	gvr("", "v1", "netnamespaces"): {
+		stub:             `{"metadata": {"name": "networkname"}, "netid": 100, "netname": "networkname"}`,
 		expectedEtcdPath: "openshift.io/registry/sdnnetnamespaces/networkname",
 	},
-	gvr("", "v1", "hostsubnets"): { // This will fail to delete because meta.name != Host but it is keyed off Host
-		stub:             `{"host": "hostname", "hostIP": "192.168.1.1", "metadata": {"name": "hs1"}, "subnet": "192.168.1.1/24"}`,
+	gvr("", "v1", "hostsubnets"): {
+		stub:             `{"host": "hostname", "hostIP": "192.168.1.1", "metadata": {"name": "hostname"}, "subnet": "192.168.1.1/24"}`,
 		expectedEtcdPath: "openshift.io/registry/sdnsubnets/hostname",
 	},
 	gvr("", "v1", "clusternetworks"): {
@@ -836,12 +835,7 @@ func (c *allClient) cleanup(all *[]cleanupData) error {
 		mapping := (*all)[i].mapping
 
 		if err := c.destroy(obj, mapping); err != nil {
-			if kubeerr.IsNotFound(err) && isInInvalidNameWhiteList(mapping) {
-				continue
-			}
 			return err
-		} else if err == nil && isInInvalidNameWhiteList(mapping) {
-			return fmt.Errorf("Object %#v with mapping %#v should fail to delete if it is in the invalid name whitelist", obj, mapping)
 		}
 	}
 	return nil
@@ -939,15 +933,6 @@ func createSerializers(config restclient.ContentConfig) (*restclient.Serializers
 	return s, nil
 }
 
-// do NOT add anything to this - doing so means you wrote something that is broken
-func isInInvalidNameWhiteList(mapping *meta.RESTMapping) bool {
-	switch mapping.GroupVersionKind.GroupVersion().WithResource(mapping.Resource) {
-	case gvr("", "v1", "netnamespaces"), gvr("", "v1", "hostsubnets"): // TODO figure out how to not whitelist these
-		return true
-	}
-	return false
-}
-
 func getFromEtcd(keys etcd.KeysAPI, path string) (*metaObject, error) {
 	response, err := keys.Get(context.Background(), path, nil)
 	if err != nil {