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

Add multi-namespace cache support #1995

Merged
merged 3 commits into from
Oct 17, 2019
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
9 changes: 5 additions & 4 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,8 @@ SKIP_DOCKER_COMMAND ?= false
GLOBAL_OPERATOR_NAMESPACE ?= elastic-system
# namespace in which the namespace operator is deployed (see config/namespace-operator)
NAMESPACE_OPERATOR_NAMESPACE ?= elastic-namespace-operators
# namespace in which the namespace operator should watch resources
MANAGED_NAMESPACE ?= default
# comma separated list of namespaces in which the namespace operator should watch resources
MANAGED_NAMESPACES ?=

## -- Security

Expand Down Expand Up @@ -170,7 +170,8 @@ go-run:
--development --operator-roles=global,namespace \
--log-verbosity=$(LOG_VERBOSITY) \
--ca-cert-validity=10h --ca-cert-rotate-before=1h \
--operator-namespace=default --namespace= \
--operator-namespace=default \
--namespaces=$(MANAGED_NAMESPACES) \
--auto-install-webhooks=false

go-debug:
Expand All @@ -184,7 +185,7 @@ go-debug:
--ca-cert-validity=10h \
--ca-cert-rotate-before=1h \
--operator-namespace=default \
--namespace= \
--namespaces=$(MANAGED_NAMESPACES) \
--auto-install-webhooks=false)

build-operator-image:
Expand Down
26 changes: 19 additions & 7 deletions cmd/manager/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import (
"github.com/spf13/cobra"
"github.com/spf13/viper"
"k8s.io/client-go/kubernetes"
"sigs.k8s.io/controller-runtime/pkg/cache"
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/runtime/signals"
)
Expand All @@ -48,7 +49,7 @@ const (
DefaultMetricPort = 0 // disabled

AutoPortForwardFlagName = "auto-port-forward"
NamespaceFlagName = "namespace"
NamespacesFlag = "namespaces"

CACertValidityFlag = "ca-cert-validity"
CACertRotateBeforeFlag = "ca-cert-rotate-before"
Expand Down Expand Up @@ -80,10 +81,10 @@ var (

func init() {

Cmd.Flags().String(
NamespaceFlagName,
"",
"namespace in which this operator should manage resources (defaults to all namespaces)",
Cmd.Flags().StringSlice(
NamespacesFlag,
nil,
"comma-separated list of namespaces in which this operator should manage resources (defaults to all namespaces)",
)
Cmd.Flags().Bool(
AutoPortForwardFlagName,
Expand Down Expand Up @@ -213,8 +214,19 @@ func execute() {
log.Info("Setting up manager")
opts := ctrl.Options{
Scheme: clientgoscheme.Scheme,
// restrict the operator to watch resources within a single namespace, unless empty
Namespace: viper.GetString(NamespaceFlagName),
}

// configure the manager cache based on the number of managed namespaces
managedNamespaces := viper.GetStringSlice(NamespacesFlag)
switch len(managedNamespaces) {
case 0:
log.Info("Operator configured to manage all namespaces")
case 1:
log.Info("Operator configured to manage a single namespace", "namespace", managedNamespaces[0])
opts.Namespace = managedNamespaces[0]
default:
log.Info("Operator configured to manage multiple namespaces", "namespaces", managedNamespaces)
opts.NewCache = cache.MultiNamespacedCacheBuilder(managedNamespaces)
}

// only expose prometheus metrics if provided a non-zero port
Expand Down
2 changes: 1 addition & 1 deletion config/e2e/batch_job.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ spec:
containers:
- name: e2e
image: {{ .E2EImage }}
imagePullPolicy: IfNotPresent
imagePullPolicy: Always
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not too familiar with how E2EImage was populated. Were we reusing image names across test runs and so they were getting cached even if they changed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Image caching is a problem if you manually run the tests with uncommitted changes.

I kept the setting because it won't affect the normal CI runs (E2E image name is unique and will require a pull from the registry anyway).

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we may have introduced IfNotPresent a while ago in order to run tests on Minikube. We didn't have a registry there, so we used to build the image in Minikube context. Meaning it was possible to use it, but not to pull it from Minikube.
The situation is different now, we do run a registry in Minikube if I remember correctly (hack/registry.sh).

args: [{{- if .TestRegex -}}"-run", "{{ .TestRegex }}",{{- end -}}"-args", "-testContextPath","/etc/e2e/testcontext.json"]
volumeMounts:
- name: test-config
Expand Down
4 changes: 2 additions & 2 deletions config/e2e/managed_namespaces.yaml
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
{{- $testRun := .TestRun -}}
{{- range .NamespaceOperators }}
{{- range .NamespaceOperator.ManagedNamespaces }}
---
apiVersion: v1
kind: Namespace
metadata:
name: {{ .ManagedNamespace }}
name: {{ . }}
labels:
test-run: {{ $testRun }}
{{- end }}
44 changes: 23 additions & 21 deletions config/e2e/namespace_operator.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -142,23 +142,21 @@ rules:
- get
- update
- patch

{{- range .NamespaceOperators }}
---
apiVersion: v1
kind: ServiceAccount
metadata:
name: {{ .Name }}
namespace: {{ .Namespace }}
name: {{ .NamespaceOperator.Name }}
namespace: {{ .NamespaceOperator.Namespace }}
labels:
test-run: {{ $testRun }}
---
# allow operator to manage resources in its namespace
apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
name: {{ .Name }}
namespace: {{ .Namespace }}
name: {{ .NamespaceOperator.Name }}
namespace: {{ .NamespaceOperator.Namespace }}
labels:
test-run: {{ $testRun }}
roleRef:
Expand All @@ -167,15 +165,18 @@ roleRef:
name: elastic-operator
subjects:
- kind: ServiceAccount
name: {{ .Name }}
namespace: {{ .Namespace }}
name: {{ .NamespaceOperator.Name }}
namespace: {{ .NamespaceOperator.Namespace }}

{{- $nsOperator := .NamespaceOperator -}}
sebgl marked this conversation as resolved.
Show resolved Hide resolved
{{ range $nsOperator.ManagedNamespaces }}
---
# allow operator to manage resources in the managed namespace
apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
name: {{ .Name }}
namespace: {{ .ManagedNamespace }}
name: {{ $nsOperator.Name }}
namespace: {{ . }}
labels:
test-run: {{ $testRun }}
roleRef:
Expand All @@ -184,34 +185,36 @@ roleRef:
name: elastic-namespace-operator
subjects:
- kind: ServiceAccount
name: {{ .Name }}
namespace: {{ .Namespace }}
name: {{ $nsOperator.Name }}
namespace: {{ $nsOperator.Namespace }}
{{- end }}

---
apiVersion: apps/v1
kind: StatefulSet
metadata:
name: {{ .Name }}
namespace: {{ .Namespace }}
name: {{ .NamespaceOperator.Name }}
namespace: {{ .NamespaceOperator.Namespace }}
labels:
control-plane: {{ .Name }}
control-plane: {{ .NamespaceOperator.Name }}
test-run: {{ $testRun }}
spec:
selector:
matchLabels:
control-plane: {{ .Name }}
serviceName: {{ .Name }}
control-plane: {{ .NamespaceOperator.Name }}
serviceName: {{ .NamespaceOperator.Name }}
template:
metadata:
labels:
control-plane: {{ .Name }}
control-plane: {{ .NamespaceOperator.Name }}
test-run: {{ $testRun }}
spec:
serviceAccountName: {{ .Name }}
serviceAccountName: {{ .NamespaceOperator.Name }}
containers:
- image: {{ $operatorImage }}
imagePullPolicy: IfNotPresent
name: manager
args: ["manager", "--namespace", "{{ .ManagedNamespace }}", "--operator-roles", "namespace"]
args: ["manager", "--namespaces", "{{ .NamespaceOperator.ManagedNamespaces | join "," }}", "--operator-roles", "namespace"]
env:
- name: OPERATOR_NAMESPACE
valueFrom:
Expand All @@ -227,4 +230,3 @@ spec:
cpu: 100m
memory: 20Mi
terminationGracePeriodSeconds: 10
{{- end }}
5 changes: 1 addition & 4 deletions config/e2e/operator_namespaces.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,10 @@ metadata:
name: {{ .GlobalOperator.Namespace }}
labels:
test-run: {{ $testRun }}

{{- range .OperatorNamespaces }}
---
apiVersion: v1
kind: Namespace
metadata:
name: {{ . }}
name: {{ .NamespaceOperator.Namespace }}
labels:
test-run: {{ $testRun }}
{{- end }}
6 changes: 2 additions & 4 deletions config/e2e/rbac.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,9 @@ subjects:
- kind: ServiceAccount
name: {{ .GlobalOperator.Name }}
namespace: {{ .GlobalOperator.Namespace }}
{{- range .NamespaceOperators }}
- kind: ServiceAccount
name: {{ .Name }}
namespace: {{ .Namespace }}
{{- end }}
name: {{ .NamespaceOperator.Name }}
namespace: {{ .NamespaceOperator.Namespace }}
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
Expand Down
17 changes: 13 additions & 4 deletions config/operator/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,23 @@ generate-global:
-e "s|<NAMESPACE>|$$NAMESPACE|g"

generate-namespace:
ifndef MANAGED_NAMESPACE
$(error MANAGED_NAMESPACE not set)
ifndef MANAGED_NAMESPACES
$(error MANAGED_NAMESPACES not set)
endif
@ for yaml in $$(LC_COLLATE=C ls namespace/*yaml); do \
@ for yaml in $(shell find namespace -type f -name '*.yaml' ! -name 'managed_ns_role_binding.template.yaml'); do \
printf "\n---\n" && cat $$yaml ; \
done | \
sed \
-e "/# /d" \
-e "s|<OPERATOR_IMAGE>|$$OPERATOR_IMAGE|g" \
-e "s|<NAMESPACE>|$$NAMESPACE|g" \
-e "s|<MANAGED_NAMESPACE>|$$MANAGED_NAMESPACE|g"
-e "s|<MANAGED_NAMESPACE_LIST>|$$MANAGED_NAMESPACES|g"

@ (MANAGED_NS_LIST=$${MANAGED_NAMESPACES//,/ }; \
for managed_ns in $$MANAGED_NS_LIST; do \
printf "\n---\n" && sed \
-e "/# /d" \
-e "s|<NAMESPACE>|$$NAMESPACE|g" \
-e "s|<MANAGED_NAMESPACE>|$$managed_ns|g" \
namespace/managed_ns_role_binding.template.yaml ;\
done)
6 changes: 3 additions & 3 deletions config/operator/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ The Elastic Operator can be deployed in 2 different modes using the following ar

* `--operator-roles`: namespace, global, webhook or all
* `--operator-namespace`: namespace the operator runs in
* `--namespace`: namespace in which resources should be watched (defaults to all namespaces)
* `--namespaces`: comma-separated list of namespaces in which resources should be watched (defaults to all namespaces)

## Deployment mode

Expand Down Expand Up @@ -34,7 +34,7 @@ OPERATOR_IMAGE=<?> NAMESPACE=<?> make generate-global | kubectl apply -f -
A namespace operator that manages resources in a given namespace.

```bash
OPERATOR_IMAGE=<?> NAMESPACE=<?> MANAGED_NAMESPACE=<?> make generate-namespace | kubectl apply -f -
OPERATOR_IMAGE=<?> NAMESPACE=<?> MANAGED_NAMESPACES=<?> make generate-namespace | kubectl apply -f -
```

## Role of each YAML file
Expand All @@ -59,4 +59,4 @@ Describes permissions for several api calls the operator needs to perform.

### role_bindings.yaml or cluster_role_bindings.yaml

Allows the operator to perform calls described in `cluster_role.yaml` in the operator namespace.
Allows the operator to perform calls described in `cluster_role.yaml` in the operator namespace.
Original file line number Diff line number Diff line change
@@ -1,18 +1,3 @@
# allow operator to manage resources in its namespace
apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
name: elastic-operator
namespace: <NAMESPACE>
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: ClusterRole
name: elastic-operator
subjects:
- kind: ServiceAccount
name: elastic-namespace-operator
namespace: <NAMESPACE>
---
# allow operator to manage resources in the managed namespace
apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
Expand Down
2 changes: 1 addition & 1 deletion config/operator/namespace/operator.template.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ spec:
containers:
- image: <OPERATOR_IMAGE>
name: manager
args: ["manager", "--namespace", "<MANAGED_NAMESPACE>", "--operator-roles", "namespace"]
args: ["manager", "--namespaces", "<MANAGED_NAMESPACE_LIST>", "--operator-roles", "namespace"]
env:
- name: OPERATOR_NAMESPACE
valueFrom:
Expand Down
14 changes: 14 additions & 0 deletions config/operator/namespace/operator_ns_role_binding.template.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# allow operator to manage resources in its namespace
apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
name: elastic-operator
namespace: <NAMESPACE>
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: ClusterRole
name: elastic-operator
subjects:
- kind: ServiceAccount
name: elastic-namespace-operator
namespace: <NAMESPACE>
4 changes: 2 additions & 2 deletions docs/operator-config.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ You can use several options to configure ECK. Unless otherwise noted, these opti
|`enable-debug-logs` |bool |false |Enables debug logs. Equivalent to `log-verbosity=1`
|`metrics-port` |int |0 |Port to use for exposing metrics in the Prometheus format. Set 0 to disable
|`operator-roles` |[]string |all |Roles this operator should assume. Valid values are namespace, global, webhook or all. Accepts multiple comma separated values. See <<{p}-ns-config>> for more information
|`namespace` |string |`""` |Namespace in which this operator should manage resources. Defaults to all namespaces. See <<{p}-ns-config>> for more information
|`namespaces` |[]string |`""` |Namespaces in which this operator should manage resources. Accepts multiple comma-separated values. Defaults to all namespaces. See <<{p}-ns-config>> for more information
|`ca-cert-validity` |duration (string) |1y |Duration representing how long before a newly created CA cert expires
|`ca-cert-rotate-before` |duration (string) |1d |Duration representing how long before expiration CA certificates should be reissued
|`cert-validity` |duration (string) |1y |Duration representing how long before a newly created TLS certificate expires
Expand Down Expand Up @@ -68,6 +68,6 @@ to:
[id="{p}-ns-config"]
=== Namespace and role configuration

The `operator-roles` and `namespace` flags have some intricacies that are worth discussing. A fully functioning operator will *require* both `global` and `namespace` roles running in the cluster (though potentially in different operator deployments). That is to say, with `--operator-roles=global,namespace` (or `--operator-roles=all`). If you want to limit the operator to a single namespace, you must set the `namespace` flag as well. For example `--operator-roles=global,namespace --namespace=my-namespace`. To have it listen on the entire cluster, you can simply omit the `namespace` flag.
The `operator-roles` and `namespaces` flags have some intricacies that are worth discussing. A fully functioning operator will *require* both `global` and `namespace` roles running in the cluster (though potentially in different operator deployments). That is to say, with `--operator-roles=global,namespace` (or `--operator-roles=all`). If you want to limit the operator to a specific set of namespaces, you must set the `namespaces` flag as well. For example `--operator-roles=global,namespace --namespaces=my-namespace1,mynamespace2`. To have it manage all namespaces, you can simply omit the `namespaces` flag.

The global role acts across namespaces and is not related to a specific deployment of the Elastic stack. The global operator deployed cluster-wide is responsible for high-level cross-cluster features.
9 changes: 1 addition & 8 deletions test/e2e/apm/association_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,6 @@ import (

// TestCrossNSAssociation tests associating Elasticsearch and an APM Server running in different namespaces.
func TestCrossNSAssociation(t *testing.T) {
// This test currently does not work in the E2E environment because each namespace has a dedicated
// controller (see https://github.com/elastic/cloud-on-k8s/issues/1438)
if !(test.Ctx().Local) {
t.SkipNow()
}

esNamespace := test.Ctx().ManagedNamespace(0)
apmNamespace := test.Ctx().ManagedNamespace(1)
name := "test-cross-ns-assoc"
Expand All @@ -46,8 +40,7 @@ func TestCrossNSAssociation(t *testing.T) {
"setup.template.settings.index.number_of_replicas": 0, // avoid ES yellow state on a 1 node ES cluster
})

test.Sequence(nil, test.EmptySteps, esBuilder, apmBuilder).
RunSequential(t)
test.Sequence(nil, test.EmptySteps, esBuilder, apmBuilder).RunSequential(t)
}

func TestAPMAssociationWithNonExistentES(t *testing.T) {
Expand Down
6 changes: 3 additions & 3 deletions test/e2e/cmd/run/eventlog.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,9 @@ func newEventLogger(client *kubernetes.Clientset, testCtx test.Context, logFileP
s := struct{}{}
el.interestingNamespaces[testCtx.E2ENamespace] = s
el.interestingNamespaces[testCtx.GlobalOperator.Namespace] = s
for _, ns := range testCtx.NamespaceOperators {
el.interestingNamespaces[ns.Namespace] = s
el.interestingNamespaces[ns.ManagedNamespace] = s
el.interestingNamespaces[testCtx.NamespaceOperator.Namespace] = s
for _, ns := range testCtx.NamespaceOperator.ManagedNamespaces {
el.interestingNamespaces[ns] = s
}

return el
Expand Down
Loading