Skip to content

Commit

Permalink
Siggy's feedback
Browse files Browse the repository at this point in the history 
Signed-off-by: Ivan Sim <[email protected]>
  • Loading branch information
Ivan Sim committed Mar 8, 2019
1 parent 777f2ce commit 6fc2128
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 158 deletions.
177 changes: 52 additions & 125 deletions pkg/inject/inject.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ type ResourceConfig struct {
podSpec *v1.PodSpec
dnsNameOverride string
proxyOutboundCapacity map[string]uint
proxyConfigOverrides map[string]string
}

// NewResourceConfig creates and initializes a ResourceConfig
Expand All @@ -73,7 +72,6 @@ func NewResourceConfig(globalConfig *config.Global, proxyConfig *config.Proxy) *
proxyConfig: proxyConfig,
podLabels: map[string]string{k8s.ControllerNSLabel: globalConfig.GetLinkerdNamespace()},
proxyOutboundCapacity: map[string]uint{},
proxyConfigOverrides: map[string]string{},
}
}

Expand Down Expand Up @@ -136,8 +134,6 @@ func (conf *ResourceConfig) GetPatch(
return nil, nil, err
}

conf.useOverridesOrDefaults()

var patch *Patch
if strings.ToLower(conf.meta.Kind) == k8s.Pod {
patch = NewPatchPod()
Expand Down Expand Up @@ -331,22 +327,20 @@ func (conf *ResourceConfig) complete(template *v1.PodTemplateSpec) {
// injectPodSpec adds linkerd sidecars to the provided PodSpec.
func (conf *ResourceConfig) injectPodSpec(patch *Patch, identity k8s.TLSIdentity) error {
var (
f = false

controlPort = conf.proxyConfigOverrides[k8s.ProxyControlPortAnnotation]
metricsPort = conf.proxyConfigOverrides[k8s.ProxyMetricsPortAnnotation]
inboundSkipPorts = conf.proxyConfigOverrides[k8s.ProxyIgnoreInboundPortsAnnotation]
outboundSkipPorts = conf.proxyConfigOverrides[k8s.ProxyIgnoreOutboundPortsAnnotation]
inboundPort = conf.proxyConfigOverrides[k8s.ProxyInboundPortAnnotation]
outboundPort = conf.proxyConfigOverrides[k8s.ProxyOutboundPortAnnotation]
controlPort = conf.getOverrideOrDefault(k8s.ProxyControlPortAnnotation)
metricsPort = conf.getOverrideOrDefault(k8s.ProxyMetricsPortAnnotation)
inboundSkipPorts = conf.getOverrideOrDefault(k8s.ProxyIgnoreInboundPortsAnnotation)
outboundSkipPorts = conf.getOverrideOrDefault(k8s.ProxyIgnoreOutboundPortsAnnotation)
inboundPort = conf.getOverrideOrDefault(k8s.ProxyInboundPortAnnotation)
outboundPort = conf.getOverrideOrDefault(k8s.ProxyOutboundPortAnnotation)
)

if len(inboundSkipPorts) > 0 {
inboundSkipPorts += ","
}
inboundSkipPorts += controlPort + "," + metricsPort

proxyUID, err := strconv.ParseInt(conf.proxyConfigOverrides[k8s.ProxyUIDAnnotation], 10, 64)
proxyUID, err := strconv.ParseInt(conf.getOverrideOrDefault(k8s.ProxyUIDAnnotation), 10, 64)
if err != nil {
return err
}
Expand Down Expand Up @@ -393,23 +387,23 @@ func (conf *ResourceConfig) injectPodSpec(patch *Patch, identity k8s.TLSIdentity
Limits: v1.ResourceList{},
}

if request := conf.proxyConfigOverrides[k8s.ProxyRequestCPUAnnotation]; request != "" {
if request := conf.getOverrideOrDefault(k8s.ProxyRequestCPUAnnotation); request != "" {
resources.Requests["cpu"] = k8sResource.MustParse(request)
}

if request := conf.proxyConfigOverrides[k8s.ProxyRequestMemoryAnnotation]; request != "" {
if request := conf.getOverrideOrDefault(k8s.ProxyRequestMemoryAnnotation); request != "" {
resources.Requests["memory"] = k8sResource.MustParse(request)
}

if limit := conf.proxyConfigOverrides[k8s.ProxyLimitCPUAnnotation]; limit != "" {
if limit := conf.getOverrideOrDefault(k8s.ProxyLimitCPUAnnotation); limit != "" {
resources.Limits["cpu"] = k8sResource.MustParse(limit)
}

if limit := conf.proxyConfigOverrides[k8s.ProxyLimitMemoryAnnotation]; limit != "" {
if limit := conf.getOverrideOrDefault(k8s.ProxyLimitMemoryAnnotation); limit != "" {
resources.Limits["memory"] = k8sResource.MustParse(limit)
}

disableExternalProfiles, err := strconv.ParseBool(conf.proxyConfigOverrides[k8s.ProxyDisableExternalProfilesAnnotation])
disableExternalProfiles, err := strconv.ParseBool(conf.getOverrideOrDefault(k8s.ProxyDisableExternalProfilesAnnotation))
if err != nil {
return err
}
Expand All @@ -432,7 +426,7 @@ func (conf *ResourceConfig) injectPodSpec(patch *Patch, identity k8s.TLSIdentity
sidecar := v1.Container{
Name: k8s.ProxyContainerName,
Image: conf.taggedProxyImage(),
ImagePullPolicy: v1.PullPolicy(conf.proxyConfigOverrides[k8s.ProxyImagePullPolicyAnnotation]),
ImagePullPolicy: v1.PullPolicy(conf.getOverrideOrDefault(k8s.ProxyImagePullPolicyAnnotation)),
TerminationMessagePolicy: v1.TerminationMessageFallbackToLogsOnError,
SecurityContext: &v1.SecurityContext{
RunAsUser: &proxyUID,
Expand All @@ -449,7 +443,7 @@ func (conf *ResourceConfig) injectPodSpec(patch *Patch, identity k8s.TLSIdentity
},
Resources: resources,
Env: []v1.EnvVar{
{Name: "LINKERD2_PROXY_LOG", Value: conf.proxyConfigOverrides[k8s.ProxyLogLevelAnnotation]},
{Name: "LINKERD2_PROXY_LOG", Value: conf.getOverrideOrDefault(k8s.ProxyLogLevelAnnotation)},
{
Name: "LINKERD2_PROXY_CONTROL_URL",
Value: fmt.Sprintf("tcp://%s:%d", controlPlaneDNS, destinationAPIPort),
Expand Down Expand Up @@ -548,14 +542,14 @@ func (conf *ResourceConfig) injectPodSpec(patch *Patch, identity k8s.TLSIdentity
initContainer := &v1.Container{
Name: k8s.InitContainerName,
Image: conf.taggedProxyInitImage(),
ImagePullPolicy: v1.PullPolicy(conf.proxyConfigOverrides[k8s.ProxyInitImagePullPolicyAnnotation]),
ImagePullPolicy: v1.PullPolicy(conf.getOverrideOrDefault(k8s.ProxyInitImagePullPolicyAnnotation)),
TerminationMessagePolicy: v1.TerminationMessageFallbackToLogsOnError,
Args: initArgs,
Args: initArgs,
SecurityContext: &v1.SecurityContext{
Capabilities: &v1.Capabilities{
Add: []v1.Capability{v1.Capability("NET_ADMIN")},
},
Privileged: &f,
Privileged: &nonRoot,
RunAsNonRoot: &nonRoot,
RunAsUser: &runAsUser,
},
Expand Down Expand Up @@ -597,160 +591,93 @@ func (conf *ResourceConfig) AddRootLabels(patch *Patch) {

func (conf *ResourceConfig) taggedProxyImage() string {
return fmt.Sprintf("%s:%s",
conf.proxyConfigOverrides[k8s.ProxyImageAnnotation],
conf.getOverrideOrDefault(k8s.ProxyImageAnnotation),
conf.globalConfig.GetVersion())
}

func (conf *ResourceConfig) taggedProxyInitImage() string {
return fmt.Sprintf("%s:%s",
conf.proxyConfigOverrides[k8s.ProxyInitImageAnnotation],
conf.getOverrideOrDefault(k8s.ProxyInitImageAnnotation),
conf.globalConfig.GetVersion())
}

func (conf *ResourceConfig) useOverridesOrDefaults() {
if !conf.KindInjectable() {
return
func (conf *ResourceConfig) getOverrideOrDefault(annotation string) string {
if value, exists := conf.podMeta.Annotations[annotation]; exists {
return value
}

log.Debugf("object annotations: %+v\n", conf.podMeta.Annotations)
log.Debugf("namespace annotations: %+v\n", conf.nsAnnotations)

useDefault := true
for _, annotation := range k8s.ProxyConfigAnnotations {
if value, exists := conf.podMeta.Annotations[annotation]; exists {
typed := conf.annotationValueType(annotation, value, !useDefault)
conf.proxyConfigOverrides[annotation] = typed
continue
}

if value, exists := conf.nsAnnotations[annotation]; exists {
typed := conf.annotationValueType(annotation, value, !useDefault)
conf.proxyConfigOverrides[annotation] = typed
continue
}

typed := conf.annotationValueType(annotation, "", useDefault)
conf.proxyConfigOverrides[annotation] = typed
if value, exists := conf.nsAnnotations[annotation]; exists {
return value
}

log.Debugf("proxy config: %+v", conf.proxyConfigOverrides)
return conf.getDefault(annotation)
}

func (conf *ResourceConfig) annotationValueType(annotation, strValue string, useDefault bool) string {
var value string
func (conf *ResourceConfig) getDefault(annotation string) string {
switch annotation {
case k8s.ProxyImageAnnotation:
if useDefault {
return conf.proxyConfig.GetProxyImage().GetImageName()
}
value = strValue
return conf.proxyConfig.GetProxyImage().GetImageName()

case k8s.ProxyImagePullPolicyAnnotation:
if useDefault {
return conf.proxyConfig.GetProxyImage().GetPullPolicy()
}
value = strValue
return conf.proxyConfig.GetProxyImage().GetPullPolicy()

case k8s.ProxyInitImageAnnotation:
if useDefault {
return conf.proxyConfig.GetProxyInitImage().GetImageName()
}
value = strValue
return conf.proxyConfig.GetProxyInitImage().GetImageName()

case k8s.ProxyInitImagePullPolicyAnnotation:
if useDefault {
return conf.proxyConfig.GetProxyInitImage().GetPullPolicy()
}
value = strValue
return conf.proxyConfig.GetProxyInitImage().GetPullPolicy()

case k8s.ProxyIgnoreInboundPortsAnnotation:
if useDefault && len(conf.proxyConfig.GetIgnoreInboundPorts()) > 0 {
for _, port := range conf.proxyConfig.GetIgnoreInboundPorts() {
portStr := strconv.FormatUint(uint64(port.GetPort()), 10)
value += portStr + ","
}
return value[:len(value)-1]
ports := []string{}
for _, port := range conf.proxyConfig.GetIgnoreInboundPorts() {
portStr := strconv.FormatUint(uint64(port.GetPort()), 10)
ports = append(ports, portStr)
}
value = strValue
return strings.Join(ports, ",")

case k8s.ProxyIgnoreOutboundPortsAnnotation:
if useDefault && len(conf.proxyConfig.GetIgnoreOutboundPorts()) > 0 {
for _, port := range conf.proxyConfig.GetIgnoreOutboundPorts() {
portStr := strconv.FormatUint(uint64(port.GetPort()), 10)
value += portStr + ","
}
return value[:len(value)-1]
ports := []string{}
for _, port := range conf.proxyConfig.GetIgnoreOutboundPorts() {
portStr := strconv.FormatUint(uint64(port.GetPort()), 10)
ports = append(ports, portStr)
}
value = strValue
return strings.Join(ports, ",")

case k8s.ProxyControlPortAnnotation:
if useDefault {
return strconv.FormatUint(uint64(conf.proxyConfig.GetControlPort().GetPort()), 10)
}
value = strValue
return strconv.FormatUint(uint64(conf.proxyConfig.GetControlPort().GetPort()), 10)

case k8s.ProxyInboundPortAnnotation:
if useDefault {
return strconv.FormatUint(uint64(conf.proxyConfig.GetInboundPort().GetPort()), 10)
}
value = strValue
return strconv.FormatUint(uint64(conf.proxyConfig.GetInboundPort().GetPort()), 10)

case k8s.ProxyMetricsPortAnnotation:
if useDefault {
return strconv.FormatUint(uint64(conf.proxyConfig.GetMetricsPort().GetPort()), 10)
}
value = strValue
return strconv.FormatUint(uint64(conf.proxyConfig.GetMetricsPort().GetPort()), 10)

case k8s.ProxyOutboundPortAnnotation:
if useDefault {
return strconv.FormatUint(uint64(conf.proxyConfig.GetOutboundPort().GetPort()), 10)
}
value = strValue
return strconv.FormatUint(uint64(conf.proxyConfig.GetOutboundPort().GetPort()), 10)

case k8s.ProxyRequestCPUAnnotation:
if useDefault {
return conf.proxyConfig.GetResource().GetRequestCpu()
}
value = strValue
return conf.proxyConfig.GetResource().GetRequestCpu()

case k8s.ProxyRequestMemoryAnnotation:
if useDefault {
return conf.proxyConfig.GetResource().GetRequestMemory()
}
value = strValue
return conf.proxyConfig.GetResource().GetRequestMemory()

case k8s.ProxyLimitCPUAnnotation:
if useDefault {
return conf.proxyConfig.GetResource().GetLimitCpu()
}
value = strValue
return conf.proxyConfig.GetResource().GetLimitCpu()

case k8s.ProxyLimitMemoryAnnotation:
if useDefault {
return conf.proxyConfig.GetResource().GetLimitMemory()
}
value = strValue
return conf.proxyConfig.GetResource().GetLimitMemory()

case k8s.ProxyUIDAnnotation:
if useDefault {
return strconv.FormatInt(conf.proxyConfig.GetProxyUid(), 10)
}
value = strValue
return strconv.FormatInt(conf.proxyConfig.GetProxyUid(), 10)

case k8s.ProxyLogLevelAnnotation:
if useDefault {
return conf.proxyConfig.GetLogLevel().GetLevel()
}
value = strValue
return conf.proxyConfig.GetLogLevel().GetLevel()

case k8s.ProxyDisableExternalProfilesAnnotation:
if useDefault {
return strconv.FormatBool(conf.proxyConfig.GetDisableExternalProfiles())
}
value = strValue
return strconv.FormatBool(conf.proxyConfig.GetDisableExternalProfiles())
}

return value
return ""
}

// ShouldInjectCLI is used by CLI inject to determine whether or not a given
Expand Down
37 changes: 26 additions & 11 deletions pkg/inject/inject_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,26 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

var proxyConfigAnnotations = []string{
k8s.ProxyImageAnnotation,
k8s.ProxyImagePullPolicyAnnotation,
k8s.ProxyInitImageAnnotation,
k8s.ProxyInitImagePullPolicyAnnotation,
k8s.ProxyControlPortAnnotation,
k8s.ProxyIgnoreInboundPortsAnnotation,
k8s.ProxyIgnoreOutboundPortsAnnotation,
k8s.ProxyInboundPortAnnotation,
k8s.ProxyMetricsPortAnnotation,
k8s.ProxyOutboundPortAnnotation,
k8s.ProxyRequestCPUAnnotation,
k8s.ProxyRequestMemoryAnnotation,
k8s.ProxyLimitCPUAnnotation,
k8s.ProxyLimitMemoryAnnotation,
k8s.ProxyUIDAnnotation,
k8s.ProxyLogLevelAnnotation,
k8s.ProxyDisableExternalProfilesAnnotation,
}

func TestOverrides(t *testing.T) {
var (
kind = "Deployment"
Expand Down Expand Up @@ -376,20 +396,15 @@ func TestOverrides(t *testing.T) {
resourceConfig.podMeta = objMeta{&metav1.ObjectMeta{}}
resourceConfig.podMeta.Annotations = podAnnotations

resourceConfig.useOverridesOrDefaults()

if len(resourceConfig.proxyConfigOverrides) != len(expectedOverrides) {
t.Errorf("Number of config overrides don't match. Expected: %d. Actual: %d", len(expectedOverrides), len(resourceConfig.proxyConfigOverrides))
}

for key, actual := range resourceConfig.proxyConfigOverrides {
expected, exist := expectedOverrides[key]
if !exist {
t.Errorf("Expected annotation %q to exist", key)
for _, annotation := range proxyConfigAnnotations {
actual := resourceConfig.getOverrideOrDefault(annotation)
expected, exists := expectedOverrides[annotation]
if !exists {
t.Errorf("Expected annotation %q to exist", annotation)
}

if !reflect.DeepEqual(expected, actual) {
t.Errorf("Annotation: %q. Expected: %v (%T). Actual: %v (%T)", key, expected, expected, actual, actual)
t.Errorf("Annotation: %q. Expected: %s. Actual: %s", annotation, expected, actual)
}
}
})
Expand Down
22 changes: 0 additions & 22 deletions pkg/k8s/labels.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -225,28 +225,6 @@ var (

// MountPathProxyConfig is the path at which the global config file is mounted
MountPathProxyConfig = MountPathBase + "/config/proxy"

// ProxyConfigAnnotations is the list of annotations that can be used to override
// proxy configurations
ProxyConfigAnnotations = []string{
ProxyImageAnnotation,
ProxyImagePullPolicyAnnotation,
ProxyInitImageAnnotation,
ProxyInitImagePullPolicyAnnotation,
ProxyControlPortAnnotation,
ProxyIgnoreInboundPortsAnnotation,
ProxyIgnoreOutboundPortsAnnotation,
ProxyInboundPortAnnotation,
ProxyMetricsPortAnnotation,
ProxyOutboundPortAnnotation,
ProxyRequestCPUAnnotation,
ProxyRequestMemoryAnnotation,
ProxyLimitCPUAnnotation,
ProxyLimitMemoryAnnotation,
ProxyUIDAnnotation,
ProxyLogLevelAnnotation,
ProxyDisableExternalProfilesAnnotation,
}
)

// CreatedByAnnotationValue returns the value associated with
Expand Down

0 comments on commit 6fc2128

Please sign in to comment.