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 2 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 \
--namespace-list=$(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= \
--namespace-list=$(MANAGED_NAMESPACES) \
--auto-install-webhooks=false)

build-operator-image:
Expand Down
26 changes: 24 additions & 2 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 @@ -49,6 +50,7 @@ const (

AutoPortForwardFlagName = "auto-port-forward"
NamespaceFlagName = "namespace"
NamespaceListFlagName = "namespace-list"
charith-elastic marked this conversation as resolved.
Show resolved Hide resolved

CACertValidityFlag = "ca-cert-validity"
CACertRotateBeforeFlag = "ca-cert-rotate-before"
Expand Down Expand Up @@ -85,6 +87,11 @@ func init() {
"",
"namespace in which this operator should manage resources (defaults to all namespaces)",
)
Cmd.Flags().StringSlice(
NamespaceListFlagName,
nil,
"List of namespaces to be managed (overrides the namespace flag)",
charith-elastic marked this conversation as resolved.
Show resolved Hide resolved
)
Cmd.Flags().Bool(
AutoPortForwardFlagName,
false,
Expand Down Expand Up @@ -213,8 +220,23 @@ 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),
}

// figure out the managed namespaces (namespace-list flag overrides the namespace flag)
managedNamespace := viper.GetString(NamespaceFlagName)
managedNamespaceList := viper.GetStringSlice(NamespaceListFlagName)
switch {
case len(managedNamespaceList) == 1:
log.Info("Operator configured to manage a single namespace", "namespace", managedNamespaceList[0])
opts.Namespace = managedNamespaceList[0]
case len(managedNamespaceList) > 1:
log.Info("Operator configured to manage multiple namespaces", "namespaces", managedNamespaceList)
opts.NewCache = cache.MultiNamespacedCacheBuilder(managedNamespaceList)
case managedNamespace == "":
log.Info("Operator configured to manage all namespaces")
default:
log.Info("Operator configured to manage a single namespace", "namespace", managedNamespace)
opts.Namespace = managedNamespace
}

// 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", "--namespace-list", "{{ .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)
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", "--namespace-list", "<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>
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