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

Allow operator to install resources to WATCH_NAMESPACE instead of requiring keda ns #203

Merged
merged 1 commit into from
Sep 12, 2023

Conversation

joelsmith
Copy link
Contributor

@joelsmith joelsmith commented Aug 24, 2023

Currently the KEDA OLM operator is hard-coded to work only in the keda namespace.

This change makes it so that whichever namespace the KEDA operator is installed to,
the operands (and required resources) will be installed to the same namespace.

The fundamental difference:
Before:

  • The operator could be installed in any namespace
  • The kedacontroller custom resource could only be installed in the keda namespace.
  • The operand controllers and necessary resources were always installed (by the operator) in the keda namespace.

After:

  • The operator can be installed in any namespace
  • The kedacontroller custom resource can only be installed in the same namespace where the operator is installed.
  • The operand controllers and necessary resources are always installed (by the operator) in the namespace where the operator is installed.

Checklist

  • Commits are signed with Developer Certificate of Origin (DCO)
  • Add tests for transform function
  • Update docs to explain the new behavior

@joelsmith joelsmith requested a review from zroubalik as a code owner August 24, 2023 21:12
@joelsmith joelsmith force-pushed the kedamain branch 2 times, most recently from 5221241 to f9ec7fd Compare August 25, 2023 00:54
@joelsmith joelsmith changed the title Allow operator to install resources to WATCH_NAMESPACE instead of requiring keda ns WIP: Allow operator to install resources to WATCH_NAMESPACE instead of requiring keda ns Aug 25, 2023
Copy link
Member

@zroubalik zroubalik left a comment

Choose a reason for hiding this comment

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

Looking good

@joelsmith joelsmith force-pushed the kedamain branch 4 times, most recently from fed2be1 to 171aca1 Compare September 7, 2023 05:03
@joelsmith joelsmith changed the title WIP: Allow operator to install resources to WATCH_NAMESPACE instead of requiring keda ns Allow operator to install resources to WATCH_NAMESPACE instead of requiring keda ns Sep 7, 2023
@joelsmith
Copy link
Contributor Author

@zroubalik thanks for the review! I had a few final tasks which are now done. (I renamed a variable as suggested by your review comment, I switched the unit tests to use ginkgo to get rid of a CI test error, and I updated the docs to match the new behavior.)

If you have time for a final review, that would be great. If not, I can have somebody from my team review it -- now that I know you're okay with the general approach.

…uiring keda ns

Signed-off-by: Joel Smith <joelsmith@redhat.com>
Copy link
Member

@zroubalik zroubalik left a comment

Choose a reason for hiding this comment

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

LGTM

Great job! I like the change in the unit tests

@joelsmith joelsmith merged commit c39d74a into kedacore:main Sep 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants