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

feat: support keystone-auth #297

Merged
merged 28 commits into from
Feb 28, 2024
Merged

feat: support keystone-auth #297

merged 28 commits into from
Feb 28, 2024

Conversation

okozachenko1203
Copy link
Member

@okozachenko1203 okozachenko1203 commented Jan 22, 2024

fix #9

@okozachenko1203
Copy link
Member Author

This has been tested on local lab

mnaser
mnaser previously requested changes Feb 9, 2024
Copy link
Member

@mnaser mnaser left a comment

Choose a reason for hiding this comment

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

I think we can take this as an opportunity to start cleaning up all of our manifests which are bundled. I think it's very difficult to manage them at the moment and I would like to suggest the following:

  1. Create a charts folder and start bundling the charts in there, if there is no chart, we build it (in this case for this)
  2. Since we already have Helm on the system, we can simply use helm template with a custom values file instead of using Jinja2
  3. With that in place, we're able to simply just bump the chart data in here and easily change the overrides by relying on values instead of raw manifest files.
  4. The templated value is what we "feed" into the ClusterResourceSet, and it also makes it easier for the long run to transition to the addon providers as well since we will have the charts ahead of time.

Can you make changes to this to accomodate for that?

@okozachenko1203
Copy link
Member Author

I think we can take this as an opportunity to start cleaning up all of our manifests which are bundled. I think it's very difficult to manage them at the moment and I would like to suggest the following:

  1. Create a charts folder and start bundling the charts in there, if there is no chart, we build it (in this case for this)
  2. Since we already have Helm on the system, we can simply use helm template with a custom values file instead of using Jinja2
  3. With that in place, we're able to simply just bump the chart data in here and easily change the overrides by relying on values instead of raw manifest files.
  4. The templated value is what we "feed" into the ClusterResourceSet, and it also makes it easier for the long run to transition to the addon providers as well since we will have the charts ahead of time.

Can you make changes to this to accomodate for that?

what if i create a helm chart and deploy like cluster-autoscaler instead of rendering templates and feeding into ClusterResourceSet configmap?

@mnaser
Copy link
Member

mnaser commented Feb 15, 2024

@okozachenko1203: this assumes that magnum-conductor can reach the cluster directly. In Atmosphere, it can. However, when users are using OSA or Kolla, that is likely not the case, so we'll have to rely on CAPO for the cluster to be reachable (especially in scenarios when the cluster proxy is running).

magnum_cluster_api/resources.py Outdated Show resolved Hide resolved
magnum_cluster_api/resources.py Outdated Show resolved Hide resolved
magnum_cluster_api/utils.py Show resolved Hide resolved
@mnaser mnaser merged commit 50a2c27 into main Feb 28, 2024
24 checks passed
@mnaser mnaser deleted the support-keystone-auth branch February 28, 2024 16:34
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.

Add support for keystone_auth_enabled
2 participants