Centralize more of the naming of GCE resources #70
Conversation
bowei
commented
Nov 4, 2017
•
bowei
commented
- Moves Namer out to its own source file.
- Improve some of the logging in the unit test.
- Lots of minor naming consistencies (look at the commits)
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
the
size/L
|
/assign @nikhiljindal |
|
/assign @nicksardo |
| @@ -28,6 +28,15 @@ import ( | |||
| ) | |||
|
|
|||
| const ( | |||
| // A single target proxy/urlmap/forwarding rule is created per loadbalancer. | |||
| // Tagged with the namespace/name of the Ingress. | |||
| targetHTTPProxyPrefix = "k8s-tp" | |||
There was a problem hiding this comment.
How about further extracting "k8s" prefix as a const (similar to what negPrefix() does)?
That way resource names can be reused by kubemci by replacing the prefix "k8s" with "mci1".
There was a problem hiding this comment.
sgtm -- on another PR though?
There was a problem hiding this comment.
yes doing in another PR sounds good. You can optionally add a TODO to track
pkg/utils/namer.go
Outdated
| // SetClusterName sets the UID/name of this cluster. | ||
| func (n *Namer) SetClusterName(name string) { | ||
| // SetCluster sets the UID/name of this cluster. | ||
| func (n *Namer) SetCluster(name string) { |
There was a problem hiding this comment.
This doesnt have to be a cluster name. It just needs to be a unique value. For ex: kubemci uses "load balancer" name instead of cluster name. How about keeping the name generic here?
pkg/utils/namer.go
Outdated
| func (n *Namer) NEGPrefix() string { | ||
| return fmt.Sprintf("k8s%s-%s", schemaVersionV1, n.GetClusterName()) | ||
| func (n *Namer) negPrefix() string { | ||
| return fmt.Sprintf("k8s%s-%s", schemaVersionV1, n.Cluster()) |
There was a problem hiding this comment.
Should the k8s here be configurable?
Namer should probably take a prefix "k8s" and use that for all resources (similar to comment above about using k8s prefix for resource names).
|
Thanks @bowei! |
|
I'm so glad I don't have to look at FirewallName and FirewallSuffix anymore. /lgtm |
k8s-ci-robot
added
the
lgtm
- Make Namer.Truncate private - Rename GetClusterName() to Cluster() - Rename GetFirewallName => Firewall() - More naming consistency cleanups - BeName() -> Backend() - BePort() -> BackendPortName() - firewall_name -> name - NEGName -> NEG - IGName() -> InstanceGroup() - FrSuffix -> FirewallRuleSuffix - FrName -> FirewallRule - Remove useless argument to FirewallRule - All calls use FirewallRuleSuffix, push back into the method. - SSLCertName -> SSLCert - Remove stutter on ClusterName, FirewallName - Reduce leakage of NEGPrefix (use IsNEG) from Namer - Cluster() -> UID()
