Skip to content

Conversation

@RedbackThomson
Copy link
Contributor

@RedbackThomson RedbackThomson commented Oct 19, 2021

Issue #, if available: aws-controllers-k8s/community#1006

Description of changes:
Use cluster introspection to determine whether the AdoptedResource CRD has been installed, starting the adoption reconciler only if they have been. This will allow cluster operators to intentionally leave the AdoptedResource uninstalled, if they wish to handle this separately.

I have tested these changes by uninstalling the AdoptedResource CRD and then running the controller. I see the following line in the logs to prove it is working:

2021-10-19T14:52:42.975-0700    INFO    adoption  AdoptedResource CRD not installed. The adoption reconciler will not be started

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@acornett21
Copy link
Contributor

@RedbackThomson Here are my test results from a 4.8 OpenShift cluster.

Without AdoptedResource applied to a cluster

2021-10-20T17:11:01.936Z        DEBUG   cache.account   created account config map      {"name": "ack-role-account-map"}
2021-10-20T17:11:01.943Z        INFO    adoption-setup  AdoptedResource CRD not installed. The adoption reconciler will not be started
2021-10-20T17:11:01.943Z        INFO    setup   starting manager        {"aws.service": "elasticache"}

Applying the AdoptedResource CRD to the cluster and then restarting the controller pod

2021-10-20T17:15:35.811Z        INFO    controller-runtime.controller   Starting Controller     {"controller": "adoptedresource"}
2021-10-20T17:15:35.811Z        INFO    controller-runtime.controller   Starting workers        {"controller": "adoptedresource", "worker count": 1}

Uninstalling the operator and leaving the AdoptedResource CRD applied to the cluster, then reinstalling the operator

2021-10-20T17:21:10.204Z        INFO    controller-runtime.controller   Starting Controller     {"controller": "adoptedresource"}
2021-10-20T17:21:10.204Z        INFO    controller-runtime.controller   Starting workers        {"controller": "adoptedresource", "worker count": 1}

I think this solves the issue and didn't see any problems with testing. If you would like something else tested please let me know.

@vijtrip2
Copy link
Contributor

/lgtm

@ack-bot ack-bot added the lgtm Indicates that a PR is ready to be merged. label Oct 25, 2021
@ack-bot
Copy link
Collaborator

ack-bot commented Oct 25, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: RedbackThomson, vijtrip2

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [RedbackThomson,vijtrip2]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ack-bot ack-bot merged commit 05271e4 into aws-controllers-k8s:main Oct 25, 2021
@RedbackThomson RedbackThomson deleted the conditional-adoption-reconciler branch October 27, 2021 19:32
vijtrip2 pushed a commit to vijtrip2/runtime that referenced this pull request Oct 27, 2021
)

Issue #, if available: aws-controllers-k8s/community#1006

Description of changes:
Use cluster introspection to determine whether the `AdoptedResource` CRD has been installed, starting the adoption reconciler only if they have been. This will allow cluster operators to intentionally leave the `AdoptedResource` uninstalled, if they wish to handle this separately.

I have tested these changes by uninstalling the `AdoptedResource` CRD and then running the controller. I see the following line in the logs to prove it is working:
```
2021-10-19T14:52:42.975-0700    INFO    adoption  AdoptedResource CRD not installed. The adoption reconciler will not be started
```

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants