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 blacklist option to Ansible watches.yaml #2374

Merged
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
- Added the [`cleanup`](./doc/cli/operator-sdk_cleanup.md) subcommand and [`run --olm`](./doc/cli/operator-sdk_run.md) to manage deployment/deletion of operators. These commands currently interact with OLM via an in-cluster registry-server created using an operator's on-disk manifests and managed by `operator-sdk`. ([#2402](https://github.com/operator-framework/operator-sdk/pull/2402), [#2441](https://github.com/operator-framework/operator-sdk/pull/2441))
- Added [`bundle create`](./doc/cli/operator-sdk_bundle_create.md) which builds, and optionally generates metadata for, [operator bundle images](https://github.com/openshift/enhancements/blob/ec2cf96/enhancements/olm/operator-registry.md). ([#2076](https://github.com/operator-framework/operator-sdk/pull/2076), [#2438](https://github.com/operator-framework/operator-sdk/pull/2438))
- Added [`bundle validate`](./doc/cli/operator-sdk_bundle_validate.md) which validates [operator bundle images](https://github.com/openshift/enhancements/blob/ec2cf96/enhancements/olm/operator-registry.md). ([#2411](https://github.com/operator-framework/operator-sdk/pull/2411))
- Added `blacklist` field to the `watches.yaml` for Ansible based operators. Blacklisted secondary resources will not be watched or cached.([#2374](https://github.com/operator-framework/operator-sdk/pull/2374))

### Changed

Expand Down
13 changes: 13 additions & 0 deletions doc/ansible/user-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ Read the [operator scope][operator-scope] documentation on how to run your opera
The Watches file contains a list of mappings from custom resources, identified
by it's Group, Version, and Kind, to an Ansible Role or Playbook. The Operator
expects this mapping file in a predefined location: `/opt/ansible/watches.yaml`
These resources, as well as child resources (determined by owner references) will
be monitored for updates and cached.

* **group**: The group of the Custom Resource that you will be watching.
* **version**: The version of the Custom Resource that you will be watching.
Expand All @@ -64,6 +66,7 @@ expects this mapping file in a predefined location: `/opt/ansible/watches.yaml`
* **manageStatus** (optional): When true (default), the operator will manage
the status of the CR generically. Set to false, the status of the CR is
managed elsewhere, by the specified role/playbook or in a separate controller.
* **blacklist**: A list of child resources (by GVK) that will not be watched or cached.

An example Watches file:

Expand Down Expand Up @@ -92,6 +95,16 @@ An example Watches file:
manageStatus: false
vars:
foo: bar

# ConfigMaps owned by a Memcached CR will not be watched or cached.
- version: v1alpha1
group: cache.example.com
kind: Memcached
role: /opt/ansible/roles/memcached
blacklist:
- group: ""
version: v1
kind: ConfigMap
```

## Customize the operator logic
Expand Down
19 changes: 18 additions & 1 deletion hack/tests/e2e-ansible.sh
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,25 @@ test_operator() {
exit 1
fi

header_text "Verify that a config map owned by the CR has been created."
if ! timeout 1m bash -c -- "until kubectl get configmap test-blacklist-watches > /dev/null 2>&1; do sleep 1; done";
then
error_text "FAIL: Unable to retrieve config map test-blacklist-watches."
operator_logs
exit 1
fi

header_text "Verify that config map requests skip the cache."
if ! kubectl logs deployment/memcached-operator -c operator | grep -e "Skipping cache lookup\".*"Path\":\"\/api\/v1\/namespaces\/default\/configmaps\/test-blacklist-watches\";
then
error_text "FAIL: test-blacklist-watches should not be accessible with the cache."
operator_logs
exit 1
fi


header_text "verify that metrics reflect cr creation"
if ! timeout 1m bash -c -- "until kubectl run -it --rm --restart=Never test-metrics --image=$metrics_test_image -- curl http://memcached-operator-metrics:8686/metrics | grep example-memcached; do sleep 1; done";
if ! timeout 60s bash -c -- "until kubectl run -it --rm --restart=Never test-metrics --image=$metrics_test_image -- curl http://memcached-operator-metrics:8686/metrics | grep example-memcached; do sleep 1; done";
then
error_text "FAIL: Failed to verify custom resource metrics"
operator_logs
Expand Down
42 changes: 36 additions & 6 deletions pkg/ansible/proxy/cache_response.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,9 @@ func (c *cacheResponseHandler) ServeHTTP(w http.ResponseWriter, req *http.Reques
break
}

if c.skipCacheLookup(r) {
log.V(2).Info("Skipping cache lookup", "resource", r)
// Skip cache for non-resource requests, not a part of skipCacheLookup for performance.
if !r.IsResourceRequest {
log.Info("Skipping cache lookup", "resource", r)
break
}

Expand All @@ -83,6 +84,11 @@ func (c *cacheResponseHandler) ServeHTTP(w http.ResponseWriter, req *http.Reques
break
}

if c.skipCacheLookup(r, k, req) {
log.Info("Skipping cache lookup", "resource", r)
break
}

// Determine if the resource is virtual. If it is then we should not attempt to use cache
isVR, err := c.apiResources.IsVirtualResource(k)
if err != nil {
Expand Down Expand Up @@ -141,9 +147,32 @@ func (c *cacheResponseHandler) ServeHTTP(w http.ResponseWriter, req *http.Reques
}

// skipCacheLookup - determine if we should skip the cache lookup
func (c *cacheResponseHandler) skipCacheLookup(r *requestfactory.RequestInfo) bool {
// check if resource is present on request
if !r.IsResourceRequest {
func (c *cacheResponseHandler) skipCacheLookup(r *requestfactory.RequestInfo, gvk schema.GroupVersionKind, req *http.Request) bool {

owner, err := getRequestOwnerRef(req)
if err != nil {
log.Error(err, "Could not get owner reference from proxy.")
return false
}
ownerGV, err := schema.ParseGroupVersion(owner.APIVersion)
if err != nil {
m := fmt.Sprintf("Could not get group version for: %v.", owner)
log.Error(err, m)
return false
}
camilamacedo86 marked this conversation as resolved.
Show resolved Hide resolved
ownerGVK := schema.GroupVersionKind{
Group: ownerGV.Group,
Version: ownerGV.Version,
Kind: owner.Kind,
}

relatedController, ok := c.cMap.Get(ownerGVK)
if !ok {
log.Info("Could not find controller for gvk.", "ownerGVK:", ownerGVK)
return false
}
if relatedController.Blacklist[gvk] {
log.Info("Skipping, because gvk is blacklisted", "GVK", gvk)
return true
}

Expand Down Expand Up @@ -184,7 +213,8 @@ func (c *cacheResponseHandler) recoverDependentWatches(req *http.Request, un *un
if typeString, ok := un.GetAnnotations()[osdkHandler.TypeAnnotation]; ok {
ownerGV, err := schema.ParseGroupVersion(ownerRef.APIVersion)
if err != nil {
log.Error(err, "Could not get ownerRef from proxy")
m := fmt.Sprintf("could not get group version for: %v", ownerGV)
log.Error(err, m)
return
}
if typeString == fmt.Sprintf("%v.%v", ownerRef.Kind, ownerGV.Group) {
Expand Down
8 changes: 7 additions & 1 deletion pkg/ansible/proxy/controllermap/controllermap.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ type Contents struct {
WatchClusterScopedResources bool
OwnerWatchMap *WatchMap
AnnotationWatchMap *WatchMap
Blacklist map[schema.GroupVersionKind]bool
}

// NewControllerMap returns a new object that contains a mapping between GVK
Expand Down Expand Up @@ -75,10 +76,15 @@ func (cm *ControllerMap) Delete(key schema.GroupVersionKind) {
}

// Store - Adds a new GVK to controller mapping
func (cm *ControllerMap) Store(key schema.GroupVersionKind, value *Contents) {
func (cm *ControllerMap) Store(key schema.GroupVersionKind, value *Contents, blacklist []schema.GroupVersionKind) {
cm.mutex.Lock()
defer cm.mutex.Unlock()
cm.internal[key] = value
// watches.go Blacklist is []schema.GroupVersionKind, which we convert to a map (better performance) for the controller.
value.Blacklist = map[schema.GroupVersionKind]bool{}
for _, blacklistGVK := range blacklist {
cm.internal[key].Blacklist[blacklistGVK] = true
}
}

// Get - Checks if GVK is already watched
Expand Down
4 changes: 2 additions & 2 deletions pkg/ansible/proxy/inject_owner.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ func (i *injectOwnerReferenceHandler) ServeHTTP(w http.ResponseWriter, req *http
isVR, err := i.apiResources.IsVirtualResource(k)
if err != nil {
// break here in case we can not understand if virtual resource or not
log.Info("Unable to determine if virual resource", "gvk", k)
log.Info("Unable to determine if virtual resource", "gvk", k)
break
}

Expand Down Expand Up @@ -127,7 +127,7 @@ func (i *injectOwnerReferenceHandler) ServeHTTP(w http.ResponseWriter, req *http
} else {
ownerGV, err := schema.ParseGroupVersion(owner.APIVersion)
if err != nil {
m := fmt.Sprintf("could not get broup version for: %v", owner)
m := fmt.Sprintf("could not get group version for: %v", owner)
log.Error(err, m)
http.Error(w, m, http.StatusBadRequest)
return
Expand Down
6 changes: 4 additions & 2 deletions pkg/ansible/proxy/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ func addWatchToController(owner kubeconfig.NamespacedOwnerReference, cMap *contr
}
ownerGV, err := schema.ParseGroupVersion(owner.APIVersion)
if err != nil {
m := fmt.Sprintf("could not get broup version for: %v", owner)
m := fmt.Sprintf("could not get group version for: %v", owner)
log.Error(err, m)
return err
}
Expand All @@ -216,7 +216,7 @@ func addWatchToController(owner kubeconfig.NamespacedOwnerReference, cMap *contr
dependentPredicate := predicates.DependentPredicateFuncs()

// Add a watch to controller
if contents.WatchDependentResources {
if contents.WatchDependentResources && !contents.Blacklist[resource.GroupVersionKind()] {
// Store watch in map
// Use EnqueueRequestForOwner unless user has configured watching cluster scoped resources and we have to
switch {
Expand Down Expand Up @@ -248,6 +248,8 @@ func addWatchToController(owner kubeconfig.NamespacedOwnerReference, cMap *contr
return err
}
}
} else {
log.Info("Resource will not be watched/cached.", "GVK", resource.GroupVersionKind())
}
return nil
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/ansible/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ func Run(flags *aoflags.AnsibleOperatorFlags) error {
WatchClusterScopedResources: w.WatchClusterScopedResources,
OwnerWatchMap: controllermap.NewWatchMap(),
AnnotationWatchMap: controllermap.NewWatchMap(),
})
}, w.Blacklist)
gvks = append(gvks, w.GroupVersionKind)
}

Expand Down
51 changes: 28 additions & 23 deletions pkg/ansible/watches/watches.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,16 +37,17 @@ var log = logf.Log.WithName("watches")
// Watch - holds data used to create a mapping of GVK to ansible playbook or role.
// The mapping is used to compose an ansible operator.
type Watch struct {
GroupVersionKind schema.GroupVersionKind `yaml:",inline"`
Playbook string `yaml:"playbook"`
Role string `yaml:"role"`
Vars map[string]interface{} `yaml:"vars"`
MaxRunnerArtifacts int `yaml:"maxRunnerArtifacts"`
ReconcilePeriod time.Duration `yaml:"reconcilePeriod"`
ManageStatus bool `yaml:"manageStatus"`
WatchDependentResources bool `yaml:"watchDependentResources"`
WatchClusterScopedResources bool `yaml:"watchClusterScopedResources"`
Finalizer *Finalizer `yaml:"finalizer"`
GroupVersionKind schema.GroupVersionKind `yaml:",inline"`
Blacklist []schema.GroupVersionKind `yaml:"blacklist"`
Playbook string `yaml:"playbook"`
Role string `yaml:"role"`
Vars map[string]interface{} `yaml:"vars"`
MaxRunnerArtifacts int `yaml:"maxRunnerArtifacts"`
ReconcilePeriod time.Duration `yaml:"reconcilePeriod"`
Finalizer *Finalizer `yaml:"finalizer"`
ManageStatus bool `yaml:"manageStatus"`
WatchDependentResources bool `yaml:"watchDependentResources"`
WatchClusterScopedResources bool `yaml:"watchClusterScopedResources"`

// Not configurable via watches.yaml
MaxWorkers int `yaml:"maxWorkers"`
Expand All @@ -63,6 +64,7 @@ type Finalizer struct {

// Default values for optional fields on Watch
var (
blacklistDefault = []schema.GroupVersionKind{}
maxRunnerArtifactsDefault = 20
reconcilePeriodDefault = "0s"
manageStatusDefault = true
Expand All @@ -81,18 +83,19 @@ var (
func (w *Watch) UnmarshalYAML(unmarshal func(interface{}) error) error {
// Use an alias struct to handle complex types
type alias struct {
Group string `yaml:"group"`
Version string `yaml:"version"`
Kind string `yaml:"kind"`
Playbook string `yaml:"playbook"`
Role string `yaml:"role"`
Vars map[string]interface{} `yaml:"vars"`
MaxRunnerArtifacts int `yaml:"maxRunnerArtifacts"`
ReconcilePeriod string `yaml:"reconcilePeriod"`
ManageStatus bool `yaml:"manageStatus"`
WatchDependentResources bool `yaml:"watchDependentResources"`
WatchClusterScopedResources bool `yaml:"watchClusterScopedResources"`
Finalizer *Finalizer `yaml:"finalizer"`
Group string `yaml:"group"`
Version string `yaml:"version"`
Kind string `yaml:"kind"`
Playbook string `yaml:"playbook"`
Role string `yaml:"role"`
Vars map[string]interface{} `yaml:"vars"`
MaxRunnerArtifacts int `yaml:"maxRunnerArtifacts"`
ReconcilePeriod string `yaml:"reconcilePeriod"`
ManageStatus bool `yaml:"manageStatus"`
WatchDependentResources bool `yaml:"watchDependentResources"`
WatchClusterScopedResources bool `yaml:"watchClusterScopedResources"`
Blacklist []schema.GroupVersionKind `yaml:"blacklist"`
Finalizer *Finalizer `yaml:"finalizer"`
}
var tmp alias

Expand All @@ -103,6 +106,7 @@ func (w *Watch) UnmarshalYAML(unmarshal func(interface{}) error) error {
tmp.MaxRunnerArtifacts = maxRunnerArtifactsDefault
tmp.ReconcilePeriod = reconcilePeriodDefault
tmp.WatchClusterScopedResources = watchClusterScopedResourcesDefault
tmp.Blacklist = blacklistDefault

if err := unmarshal(&tmp); err != nil {
return err
Expand Down Expand Up @@ -136,7 +140,7 @@ func (w *Watch) UnmarshalYAML(unmarshal func(interface{}) error) error {
w.WatchClusterScopedResources = tmp.WatchClusterScopedResources
w.Finalizer = tmp.Finalizer
w.AnsibleVerbosity = getAnsibleVerbosity(gvk, ansibleVerbosityDefault)

w.Blacklist = tmp.Blacklist
return nil
}

Expand Down Expand Up @@ -172,6 +176,7 @@ func (w *Watch) Validate() error {
func New(gvk schema.GroupVersionKind, role, playbook string, vars map[string]interface{}, finalizer *Finalizer) *Watch {
reconcilePeriod, _ := time.ParseDuration(reconcilePeriodDefault)
return &Watch{
Blacklist: blacklistDefault,
GroupVersionKind: gvk,
Playbook: playbook,
Role: role,
Expand Down
12 changes: 12 additions & 0 deletions test/ansible-memcached/tasks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -65,3 +65,15 @@
metadata:
name: testing-foo
when: "'project.openshift.io' in api_groups"

- name: Create ConfigMap to test blacklisted watches
k8s:
definition:
kind: ConfigMap
apiVersion: v1
metadata:
name: test-blacklist-watches
namespace: "{{ meta.namespace }}"
data:
arbitrary: afdasdfsajsafj
state: present
4 changes: 4 additions & 0 deletions test/ansible-memcached/watches-finalizer.yaml
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
finalizer:
name: finalizer.ansible.example.com
role: /opt/ansible/roles/memfin
blacklist:
- group: ""
version: v1
kind: ConfigMap