You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2022/05/20 10:37:17 UTC

[GitHub] [apisix-helm-chart] NMichas opened a new issue, #291: A more flexible apisix.serviceNamespace for ingress chart

NMichas opened a new issue, #291:
URL: https://github.com/apache/apisix-helm-chart/issues/291

   Hello,
   
   Currently the `values.yaml` of `apisix-ingress-controller` includes `config.apisix.serviceNamespace=ingress-apisix`. Users can override this during installation by passing `--set config.apisix.serviceNamespace`.
   
   Wouldn't it be a little more flexible having `config.apisix.serviceNamespace` by default using the currently selected kubectl/release namespace and only if it is explicitly set by the user then to point to a different namespace?
   
   The above suggestion only requires two small, backwards-compatible changes in `configmap.yaml` and `deployment.yaml` files, for which I could send a PR if you want.
   
   Somewhat relevant issues: #176 #206 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-helm-chart] tokers commented on issue #291: A more flexible apisix.serviceNamespace for ingress chart

Posted by GitBox <gi...@apache.org>.
tokers commented on issue #291:
URL: https://github.com/apache/apisix-helm-chart/issues/291#issuecomment-1135291598

   > So, PR is now updated:
   > 
   > * Ingress
   >   Ingress chart can now be installed in the same namespace as the admin service without having to pass `--set` parameters to Helm. In addition, ingress' URL of the admin service can be created automatically if missing, prefixed with the release name. From `values.yaml`:
   >   ```
   >   apisix:
   >       # The name of the APISIX admin service. Leave it empty to use <Release name>-apisix-admin.
   >       serviceName: ""
   >       # The namespace of APISIX admin service. Leave it empty to use the release namespace.
   >       serviceNamespace: ""
   >   ```
   > * Dashboard
   >   The URL of etcd is now automatically prefixed with the release name, when left empty. This allows the dashboard to be installed into a custom namespace without having to pass `--set` parameters to Helm. From `values.yaml`:
   >   ```
   >   config:
   >     conf:
   >       ...
   >       etcd:
   >         # Supports defining multiple etcd host addresses for an etcd cluster.
   >         # Leave it undefined to use <Release name>-etcd:2379 as an endpoint.
   >         # endpoints:
   >         #   - apisix-etcd:2379
   >   ```
   > 
   > The above PR allows you to:
   > 
   > * Install admin with dashboard and/or ingress in one go on the same namespace using any Helm release name (i.e. `helm install myapp https://charts.apiseven.com --set dashboard.enabled=true --set ingress-controller.enabled=true` should now work anywhere).
   > * Create a meta-chart for your own project having all three APISIX charts as dependencies, without having to specify any additional configuration for the namespace or the Helm release name (that's my use case).
   > * If you wish so, you can still manually specify `apisix.serviceName`, `apisix.serviceNamespace`, and `config.conf.etcd.endpoints` via `--set`; in that case the behaviour of this PR's charts is the same as before this PR (in fact, manual configuration is still necessary if you decide to install the three charts individually).
   > 
   > Please let me know if you see anything wrong and I'm happy to modify it.
   
   LGTM


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-helm-chart] juzhiyuan commented on issue #291: A more flexible apisix.serviceNamespace for ingress chart

Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on issue #291:
URL: https://github.com/apache/apisix-helm-chart/issues/291#issuecomment-1134355893

   > @tokers yes, indeed.
   > 
   > @tao12345666333 exactly, in my case, I'm creating a meta-chart for our stack which includes APISIX. I'd like my users to be able to do a Helm install of our stack without having to specify additional parameters for the third-party dependencies of our chart.
   > 
   > Here's my PR for review: #292
   
   Thanks! After all CI passed, we will review :)
   
   BTW @NMichas, except for GitHub, we also have the `#apisix` Slack channel with 600+ members under the Apache Software Foundation's workspace. Welcome to join in via https://apisix.apache.org/docs/general/join/ Most of Apache APISIX users and maintainers are here.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-helm-chart] NMichas commented on issue #291: A more flexible apisix.serviceNamespace for ingress chart

Posted by GitBox <gi...@apache.org>.
NMichas commented on issue #291:
URL: https://github.com/apache/apisix-helm-chart/issues/291#issuecomment-1134602546

   @tokers I tested a bit more and realised `serviceName` exhibits a similar behaviour. I'm now testing an updated PR taking into account both `serviceName` and `serviceNamespace`. I shall be able to submit it in the next few hours, so you can discard my previous PR for the time being.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-helm-chart] tao12345666333 commented on issue #291: A more flexible apisix.serviceNamespace for ingress chart

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on issue #291:
URL: https://github.com/apache/apisix-helm-chart/issues/291#issuecomment-1134968917

   SGTM


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-helm-chart] tokers commented on issue #291: A more flexible apisix.serviceNamespace for ingress chart

Posted by GitBox <gi...@apache.org>.
tokers commented on issue #291:
URL: https://github.com/apache/apisix-helm-chart/issues/291#issuecomment-1134069107

   > by default using the currently selected kubectl/release namespace
   
   You mean the namespace that when users run the `helm` command?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-helm-chart] NMichas commented on issue #291: A more flexible apisix.serviceNamespace for ingress chart

Posted by GitBox <gi...@apache.org>.
NMichas commented on issue #291:
URL: https://github.com/apache/apisix-helm-chart/issues/291#issuecomment-1134358595

   Thanks, I've already joined you on Slack :)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-helm-chart] NMichas commented on issue #291: A more flexible apisix.serviceNamespace for ingress chart

Posted by GitBox <gi...@apache.org>.
NMichas commented on issue #291:
URL: https://github.com/apache/apisix-helm-chart/issues/291#issuecomment-1134282499

   @tokers yes, indeed.
   
   @tao12345666333 exactly, in my case, I'm creating a meta-chart for our stack which includes APISIX. I'd like my users to be able to do a Helm install of our stack without having to specify additional parameters for the third-party dependencies of our chart.
   
   Here's my PR for review:
   https://github.com/apache/apisix-helm-chart/pull/292


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-helm-chart] NMichas commented on issue #291: A more flexible apisix.serviceNamespace for ingress chart

Posted by GitBox <gi...@apache.org>.
NMichas commented on issue #291:
URL: https://github.com/apache/apisix-helm-chart/issues/291#issuecomment-1134875260

   So, PR is now updated:
   
   - Ingress
   Ingress chart can now be installed in the same namespace as the admin service without having to pass `--set` parameters to Helm. In addition, ingress' URL of the admin service can be created automatically if missing, prefixed with the release name. From `values.yaml`:
   
     ```
     apisix:
         # The name of the APISIX admin service. Leave it empty to use <Release name>-apisix-admin.
         serviceName: ""
         # The namespace of APISIX admin service. Leave it empty to use the release namespace.
         serviceNamespace: ""
     ```
   
   - Dashboard
   The URL of etcd is now automatically prefixed with the release name, when left empty. This allows the dashboard to be installed into a custom namespace without having to pass `--set` parameters to Helm. From `values.yaml`:
   
     ```
     config:
       conf:
         ...
         etcd:
           # Supports defining multiple etcd host addresses for an etcd cluster.
           # Leave it undefined to use <Release name>-etcd:2379 as an endpoint.
           # endpoints:
           #   - apisix-etcd:2379
     ```
   
   The above PR allows you to:
   - Install admin with dashboard and/or ingress in one go on the same namespace using any Helm release name (i.e. `helm install myapp https://charts.apiseven.com --set dashboard.enabled=true --set ingress-controller.enabled=true` should now work anywhere).
   - Create a meta-chart for your own project having all three APISIX charts as dependencies, without having to specify any additional configuration for the namespace or the Helm release name (that's my use case).
   - If you wish so, you can still manually specify `apisix.serviceName`, `apisix.serviceNamespace`, and `config.conf.etcd.endpoints` via `--set`; in that case the behaviour of this PR's charts is the same as before this PR (in fact, manual configuration is still necessary if you decide to install the three charts individually).
   
   Please let me know if you see anything wrong and I'm happy to modify it.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [apisix-helm-chart] tao12345666333 commented on issue #291: A more flexible apisix.serviceNamespace for ingress chart

Posted by GitBox <gi...@apache.org>.
tao12345666333 commented on issue #291:
URL: https://github.com/apache/apisix-helm-chart/issues/291#issuecomment-1134093698

   I think this is valuable so that users can install APISIX and APISIX Ingress in the same namepace without specifying additional parameters.
   
   https://github.com/apache/apisix-helm-chart/pull/284  There is a modification, but only the Release namespace is used. It does not solve all scenarios https://github.com/apache/apisix-helm-chart/pull/284#issuecomment-1126648297 
   
   Adding compatible configuration is very good, welcome to submit PR


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org