Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Metrics configuration #(1/4) #525

Merged
merged 1 commit into from
Jun 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions chart/k8gb/templates/operator.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -125,3 +125,5 @@ spec:
value: "true"
- name: SPLIT_BRAIN_CHECK
value: {{ quote .Values.k8gb.splitBrainCheck }}
- name: METRICS_ADDRESS
value: {{ .Values.k8gb.metricsAddress }}
1 change: 1 addition & 0 deletions chart/k8gb/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ k8gb:
format: simple # log format (simple,json)
level: info # log level (panic,fatal,error,warn,info,debug,trace)
splitBrainCheck: false
metricsAddress: "0.0.0.0:8080"

externaldns:
image: k8s.gcr.io/external-dns/external-dns:v0.7.5
Expand Down
2 changes: 2 additions & 0 deletions controllers/depresolver/depresolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,8 @@ type Config struct {
CoreDNSExposed bool
// Log configuration
Log Log
// MetricsAddress in format address:port where address can be empty, IP address, or hostname, default: 0.0.0.0:8080
MetricsAddress string
// route53Enabled hidden. EdgeDNSType defines all enabled Enabled types
route53Enabled bool
// ns1Enabled flag
Expand Down
27 changes: 27 additions & 0 deletions controllers/depresolver/depresolver_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package depresolver

import (
"fmt"
"strconv"
"strings"

"github.com/AbsaOSS/gopkg/env"
Expand Down Expand Up @@ -51,6 +52,7 @@ const (
LogFormatKey = "LOG_FORMAT"
LogNoColorKey = "NO_COLOR"
SplitBrainCheckKey = "SPLIT_BRAIN_CHECK"
MetricsAddressKey = "METRICS_ADDRESS"
somaritane marked this conversation as resolved.
Show resolved Hide resolved
)

// ResolveOperatorConfig executes once. It reads operator's configuration
Expand Down Expand Up @@ -81,6 +83,7 @@ func (dr *DependencyResolver) ResolveOperatorConfig() (*Config, error) {
dr.config.Log.Level, _ = zerolog.ParseLevel(strings.ToLower(env.GetEnvAsStringOrFallback(LogLevelKey, zerolog.InfoLevel.String())))
dr.config.Log.Format = parseLogOutputFormat(strings.ToLower(env.GetEnvAsStringOrFallback(LogFormatKey, SimpleFormat.String())))
dr.config.Log.NoColor = env.GetEnvAsBoolOrFallback(LogNoColorKey, false)
dr.config.MetricsAddress = env.GetEnvAsStringOrFallback(MetricsAddressKey, "0.0.0.0:8080")
dr.config.SplitBrainCheck = env.GetEnvAsBoolOrFallback(SplitBrainCheckKey, false)
dr.config.EdgeDNSType, recognizedDNSTypes = getEdgeDNSType(dr.config)
dr.errorConfig = dr.validateConfig(dr.config, recognizedDNSTypes)
Expand Down Expand Up @@ -193,9 +196,33 @@ func (dr *DependencyResolver) validateConfig(config *Config, recognizedDNSTypes
return fmt.Errorf("error for geo tag: %s. %s in ns name %s", geoTag, err, nsName)
}
}

mHost, mPort, err := parseMetricsAddr(config.MetricsAddress)
if err != nil {
return fmt.Errorf("invalid %s: expecting MetricsAddress in form {host}:port (%s)", MetricsAddressKey, err)
}
err = field(MetricsAddressKey, mHost).matchRegexps(hostNameRegex, ipAddressRegex).err
if err != nil {
return err
}
err = field(MetricsAddressKey, mPort).isLessOrEqualTo(65535).isHigherThan(1024).err
if err != nil {
return err
}
return nil
}

func parseMetricsAddr(metricsAddr string) (host string, port int, err error) {
ma := strings.Split(metricsAddr, ":")
if len(ma) != 2 {
err = fmt.Errorf("invalid format {host}:port (%s)", metricsAddr)
return
}
host = ma[0]
port, err = strconv.Atoi(ma[1])
return
}

// getEdgeDNSType contains logic retrieving EdgeDNSType.
func getEdgeDNSType(config *Config) (EdgeDNSType, []EdgeDNSType) {
recognized := make([]EdgeDNSType, 0)
Expand Down
98 changes: 97 additions & 1 deletion controllers/depresolver/depresolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ var predefinedConfig = Config{
DNSZone: "cloud.example.com",
K8gbNamespace: "k8gb",
SplitBrainCheck: true,
MetricsAddress: "0.0.0.0:8080",
Infoblox: Infoblox{
"Infoblox.host.com",
"0.0.3",
Expand Down Expand Up @@ -176,6 +177,7 @@ func TestResolveConfigWithoutEnvVarsSet(t *testing.T) {
defaultConfig.Log.Level = zerolog.InfoLevel
defaultConfig.Log.Format = SimpleFormat
defaultConfig.Log.NoColor = false
defaultConfig.MetricsAddress = "0.0.0.0:8080"
resolver := NewDependencyResolver()
// act
config, err := resolver.ResolveOperatorConfig()
Expand Down Expand Up @@ -1557,6 +1559,99 @@ func TestNsServerNamesWithExceededDNSNameSize(t *testing.T) {
assert.Len(t, config.GetExternalClusterNSNames()[customConfig.ExtClustersGeoTags[1]], 253)
}

func TestMetricsAddressIsValid(t *testing.T) {
defer cleanup()
expected := predefinedConfig
expected.MetricsAddress = "0.0.0.0:9091"
arrangeVariablesAndAssert(t, expected, assert.NoError)
}

func TestMetricsAddressPortLimitExceed(t *testing.T) {
defer cleanup()
expected := predefinedConfig
expected.MetricsAddress = "0.0.0.0:80091"
arrangeVariablesAndAssert(t, expected, assert.Error)
}

func TestMetricsAddressPortLimitUnder(t *testing.T) {
defer cleanup()
expected := predefinedConfig
expected.MetricsAddress = "0.0.0.0:1024"
arrangeVariablesAndAssert(t, expected, assert.Error)
}

func TestMetricsAddressInvalidPort(t *testing.T) {
defer cleanup()
expected := predefinedConfig
expected.MetricsAddress = "0.0.0.0:808x"
arrangeVariablesAndAssert(t, expected, assert.Error)
}

func TestMetricsAddressInvalidHost(t *testing.T) {
defer cleanup()
expected := predefinedConfig
expected.MetricsAddress = "x.y.z??:8080"
arrangeVariablesAndAssert(t, expected, assert.Error)
}

func TestMetricsAddressWrongFormatMultipleParts(t *testing.T) {
defer cleanup()
expected := predefinedConfig
expected.MetricsAddress = "0.0.0.0:8080:9090"
arrangeVariablesAndAssert(t, expected, assert.Error)
}

func TestMetricsAddressWrongFormatSinglePart(t *testing.T) {
defer cleanup()
expected := predefinedConfig
expected.MetricsAddress = "0.0.0.08080"
arrangeVariablesAndAssert(t, expected, assert.Error)
}

func TestMetricsAddressInvalidEnvVariable(t *testing.T) {
defer cleanup()
expected := predefinedConfig
expected.MetricsAddress = "invalid"
arrangeVariablesAndAssert(t, expected, assert.Error)
}

func TestMetricsAddressEmptyHost(t *testing.T) {
defer cleanup()
expected := predefinedConfig
expected.MetricsAddress = ":8080"
arrangeVariablesAndAssert(t, expected, assert.NoError)
}

func TestMetricsAddressWithValidHostName(t *testing.T) {
defer cleanup()
expected := predefinedConfig
expected.MetricsAddress = "address.com:8080"
arrangeVariablesAndAssert(t, expected, assert.NoError)
}

func TestMetricsAddressEmptyEnvVariable(t *testing.T) {
// arrange
defer cleanup()
expected := predefinedConfig
expected.MetricsAddress = ""
configureEnvVar(expected)
resolver := NewDependencyResolver()
// act
config, err := resolver.ResolveOperatorConfig()
// assert
assert.NoError(t, err)
assert.Equal(t, "0.0.0.0:8080", config.MetricsAddress)
}

func TestMetricsAddressEnvVarIsUnset(t *testing.T) {
// arrange
defer cleanup()
expected := predefinedConfig
expected.MetricsAddress = "0.0.0.0:8080"
// act,assert
arrangeVariablesAndAssert(t, expected, assert.NoError, MetricsAddressKey)
}

// arrangeVariablesAndAssert sets string environment variables and asserts `expected` argument with
// ResolveOperatorConfig() output. The last parameter unsets the values
func arrangeVariablesAndAssert(t *testing.T, expected Config,
Expand All @@ -1580,7 +1675,7 @@ func cleanup() {
for _, s := range []string{ReconcileRequeueSecondsKey, ClusterGeoTagKey, ExtClustersGeoTagsKey, EdgeDNSZoneKey, DNSZoneKey, EdgeDNSServerKey,
EdgeDNSServerPortKey, Route53EnabledKey, NS1EnabledKey, InfobloxGridHostKey, InfobloxVersionKey, InfobloxPortKey, InfobloxUsernameKey,
InfobloxPasswordKey, OverrideFakeInfobloxKey, K8gbNamespaceKey, CoreDNSExposedKey, InfobloxHTTPRequestTimeoutKey,
InfobloxHTTPPoolConnectionsKey, LogLevelKey, LogFormatKey, LogNoColorKey, SplitBrainCheckKey} {
InfobloxHTTPPoolConnectionsKey, LogLevelKey, LogFormatKey, LogNoColorKey, MetricsAddressKey, SplitBrainCheckKey} {
if os.Unsetenv(s) != nil {
panic(fmt.Errorf("cleanup %s", s))
}
Expand Down Expand Up @@ -1610,6 +1705,7 @@ func configureEnvVar(config Config) {
_ = os.Setenv(LogLevelKey, config.Log.Level.String())
_ = os.Setenv(LogFormatKey, config.Log.Format.String())
_ = os.Setenv(LogNoColorKey, strconv.FormatBool(config.Log.NoColor))
_ = os.Setenv(MetricsAddressKey, config.MetricsAddress)
_ = os.Setenv(SplitBrainCheckKey, strconv.FormatBool(config.SplitBrainCheck))
}

Expand Down
10 changes: 10 additions & 0 deletions controllers/depresolver/depresolver_validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,16 @@ func (v *validator) isLessOrEqualTo(num int) *validator {
return v
}

func (v *validator) isHigherThan(num int) *validator {
if v.err != nil {
return v
}
if v.intValue <= num {
v.err = fmt.Errorf(`'%s' is higher than '%v'`, v.name, num)
}
return v
}

func (v *validator) isNotEqualTo(value string) *validator {
if v.err != nil {
return v
Expand Down
2 changes: 1 addition & 1 deletion controllers/gslb_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ func (r *GslbReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.

// SetupWithManager configures controller manager
func (r *GslbReconciler) SetupWithManager(mgr ctrl.Manager) error {
// Figure out Gslb resource name to Reconcile when non controlled Endpoint is updated
// Figure out Gslb resource name to Reconcile when non controlled Name is updated

endpointMapHandler := handler.EnqueueRequestsFromMapFunc(
func(a client.Object) []reconcile.Request {
Expand Down
13 changes: 2 additions & 11 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ limitations under the License.
package main

import (
"flag"
"os"

str "github.com/AbsaOSS/gopkg/strings"
Expand Down Expand Up @@ -49,15 +48,7 @@ func init() {
}

func main() {
var metricsAddr string
var enableLeaderElection bool
var f *dns.ProviderFactory
flag.StringVar(&metricsAddr, "metrics-addr", ":8080", "The address the metric endpoint binds to.")
flag.BoolVar(&enableLeaderElection, "enable-leader-election", false,
"Enable leader election for controller manager. "+
"Enabling this will ensure there is only one active controller manager.")
flag.Parse()

resolver := depresolver.NewDependencyResolver()
config, err := resolver.ResolveOperatorConfig()
// Initialize desired log or default log in case of configuration failed.
Expand All @@ -75,9 +66,9 @@ func main() {

mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{
Scheme: runtimescheme,
MetricsBindAddress: metricsAddr,
MetricsBindAddress: config.MetricsAddress,
Port: 9443,
LeaderElection: enableLeaderElection,
LeaderElection: false,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, we never tested it, but what if somebody would like to try to execute multiple k8gb controllers with leader election mechanism in the future? currently it is optional but here it is becoming hardcoded.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

enableLeaderElection was accessible through an argument of executable. Making such functionality available would mean passing the argument to the controller pod. Till now it was always false anyway.

What we can do is create an issue, expose an environment variable and test.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kuritka ok let's log the issue then

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

opened #526

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

even more, I think it requires to have locking to be implemented.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point @k0da. Feel free and add your comments by #526 please.

LeaderElectionID: "8020e9ff.absa.oss",
})
if err != nil {
Expand Down