Re: [PR] Enable defining a network as redundant during restart through the UI [cloudstack]
JoaoJandre commented on PR #7405: URL: https://github.com/apache/cloudstack/pull/7405#issuecomment-2077178685 > @DaanHoogland @borisstoyanov @weizhouapache @andrijapanicsb @rohityadavcloud @alexandremattioli @NuxRo > > I see that the comunity has mixed opinions regarding this functionality, however, I still believe we should maintain conformity on the API and the UI. In order to achieve this without raising further discussions, I propose that we create a global configuration with default value `false` that dictates whether this parameter can be used (both on the UI and the API). If set to false, it will not show up on the UI and trying to use it on the API will generate an exception. > > This would give operators the flexibility to remove this feature from the backend if they see it as harmful, or leave it both on the backend and the frontend if they see it as helpful. > > Would this implementation be enough to offset your concerns regarding this feature? @GaOrtiga The problem with the proposed solution is that this is **breaking backwards compatibility**. When users update to 4.20, they'll start to get an exception when informing this parameter on the API. This kind of change should only be made when changing major versions. Last month I made a proposal for a release schedule that would let us do this kind of change (see https://lists.apache.org/thread/o6o9h3qp8gqrpq4v7o81tl6vp51tkjhg) but sadly it got almost no traction; there is a new discussion thread about it now (https://github.com/apache/cloudstack/discussions/8970), I hope we can advance it this time. As the project stands right now, this proposal should not be accepted. -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Enable defining a network as redundant during restart through the UI [cloudstack]
GaOrtiga commented on PR #7405: URL: https://github.com/apache/cloudstack/pull/7405#issuecomment-2073094359 @DaanHoogland @borisstoyanov @weizhouapache @andrijapanicsb @rohityadavcloud @alexandremattioli @NuxRo I see that the comunity has mixed opinions regarding this functionality, however, I still believe we should maintain conformity on the API and the UI. In order to achieve this without raising further discussions, I propose that we create a global configuration with default value `false` that dictates whether this parameter can be used (both on the UI and the API). If set to false, it will not show up on the UI and trying to use it on the API will generate an exception. This would give operators the flexibility to remove this feature from the backend if they see it as harmful, or leave it both on the backend and the frontend if they see it as helpful. Would this implementation be enough to offset your concerns regarding this feature? -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Enable defining a network as redundant during restart through the UI [cloudstack]
rohityadavcloud commented on PR #7405: URL: https://github.com/apache/cloudstack/pull/7405#issuecomment-1926523115 A global setting or UI flag based (via config.json) option is okay but then we should make this feature opt-in, and disabled by default. Admins/operators who want this feature can enable it for the UI. -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Enable defining a network as redundant during restart through the UI [cloudstack]
DaanHoogland commented on PR #7405: URL: https://github.com/apache/cloudstack/pull/7405#issuecomment-1838643947 > > > Instead of a global setting or a flag in the config.json I propose that we wait for #6918 to be merged and create a network scope setting for this feature, since it will give operators more flexibility to enable this to each network individually. > > > @DaanHoogland @borisstoyanov @weizhouapache @andrijapanicsb @rohityadavcloud @alexandremattioli @NuxRo > > > Do you have any concerns about this strategy, or can I advance with this implementation when the PR gets merged? > > > > > > I think it is kind of strange to allow to change a network to redundant based on a setting of the network. How do you see that @GaOrtiga ? > > I believe it gives operators the flexibility to turn this feature on/off individually for each user, while keeping the current behaviour for the operators that do not want to use it, so I see it as a positive implementation. However, I am open to feedback. The strange thing, to me is that if it is a setting of the network, the operator would have to inable it for each network that a user creates. instead of at a higher level (zone or domain or some other enitity). I would think if the user requests it you just make it redundant, no further action. -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Enable defining a network as redundant during restart through the UI [cloudstack]
GaOrtiga commented on PR #7405: URL: https://github.com/apache/cloudstack/pull/7405#issuecomment-1838525177 > > Instead of a global setting or a flag in the config.json I propose that we wait for #6918 to be merged and create a network scope setting for this feature, since it will give operators more flexibility to enable this to each network individually. > > @DaanHoogland @borisstoyanov @weizhouapache @andrijapanicsb @rohityadavcloud @alexandremattioli @NuxRo > > Do you have any concerns about this strategy, or can I advance with this implementation when the PR gets merged? > > I think it is kind of strange to allow to change a network to redundant based on a setting of the network. How do you see that @GaOrtiga ? I believe it gives operators the flexibility to turn this feature on/off individually for each user, while keeping the current behaviour for the operators that do not want to use it, so I see it as a positive implementation. However, I am open to feedback. -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Enable defining a network as redundant during restart through the UI [cloudstack]
DaanHoogland commented on PR #7405: URL: https://github.com/apache/cloudstack/pull/7405#issuecomment-1826347221 > Instead of a global setting or a flag in the config.json I propose that we wait for #6918 to be merged and create a network scope setting for this feature, since it will give operators more flexibility to enable this to each network individually. > > @DaanHoogland @borisstoyanov @weizhouapache @andrijapanicsb @rohityadavcloud @alexandremattioli @NuxRo > > Do you have any concerns about this strategy, or can I advance with this implementation when the PR gets merged? I think it is kind of strange to allow to change a network to redundant based on a setting of the network. How do you see that @GaOrtiga ? -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Enable defining a network as redundant during restart through the UI [cloudstack]
GaOrtiga commented on PR #7405: URL: https://github.com/apache/cloudstack/pull/7405#issuecomment-1825964242 Instead of a global setting or a flag in the config.json I propose that we wait for https://github.com/apache/cloudstack/pull/6918 to be merged and create a network scope setting for this feature, since it will give operators more flexibility to enable this to each network individually. @DaanHoogland @borisstoyanov @weizhouapache @andrijapanicsb @rohityadavcloud @alexandremattioli @NuxRo Do you have any concerns about this strategy, or can I advance with this implementation when the PR gets merged? -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Enable defining a network as redundant during restart through the UI [cloudstack]
weizhouapache commented on PR #7405: URL: https://github.com/apache/cloudstack/pull/7405#issuecomment-1804723604 > > Agree with what Rohit said, this was enabled some releases ago, and it created issues for the Operators. It should be "impossible" by default, with the option to make it configured/enabled if the operator really needs it. > > +1 what Andrija said. Not against having the feature, but let's make it depend on a global option or something, or a network offering like @borisstoyanov suggested. > We should the same feature that exists in a VPC as well to be honest. "remove" after "should"? In my opinion, redundant VR support is a very good feature, it works pretty well, if the environment has configured correctly and VRRP works without any issue. However, considering the various network devices, multiple upstream routers, VMware switch or openvswitch, etc. I have to say network configuration is too complicated . I have the experience in the past that redundant VRs work well for some years but suddenly few (of many) networks have two MASTER VRs which lead to some downtime of user VMs. The issue is not reproducible. After troubleshooting for some months, the root cause was found, it was caused by a small configuration on one of the two core switches. Be careful to enable redundant VRs in production... -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Enable defining a network as redundant during restart through the UI [cloudstack]
NuxRo commented on PR #7405: URL: https://github.com/apache/cloudstack/pull/7405#issuecomment-1804647654 > Agree with what Rohit said, this was enabled some releases ago, and it created issues for the Operators. It should be "impossible" by default, with the option to make it configured/enabled if the operator really needs it. +1 what Andrija said. Not against having the feature, but let's make it depend on a global option or something, or a network offering like @borisstoyanov suggested. We should the same feature that exists in a VPC as well to be honest. -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Enable defining a network as redundant during restart through the UI [cloudstack]
alexandremattioli commented on PR #7405: URL: https://github.com/apache/cloudstack/pull/7405#issuecomment-1804412272 -1 on adding this to the UI. Until the fundamental issues with VRRP are solved this feature should be avoided at all costs. -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Enable defining a network as redundant during restart through the UI [cloudstack]
andrijapanicsb commented on PR #7405: URL: https://github.com/apache/cloudstack/pull/7405#issuecomment-1792315195 Agree with what Rohit said, this was enabled some releases ago, and it created issues for the Operators. It should be "impossible" by default, with the option to make it configured/enabled if the operator really needs 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Enable defining a network as redundant during restart through the UI [cloudstack]
rohityadavcloud commented on PR #7405: URL: https://github.com/apache/cloudstack/pull/7405#issuecomment-1792313989 @GaOrtiga the only way could be to introduce a feature flag in config.json (in UI) and have it disabled by default, when enabled you can pass the options in UI to show the elements. Could you review if this approach works for your use-case or close the PR. Thanks. -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Enable defining a network as redundant during restart through the UI [cloudstack]
DaanHoogland commented on PR #7405: URL: https://github.com/apache/cloudstack/pull/7405#issuecomment-1787370340 @GaOrtiga how do you want to move forward with this? (moving it to 4.20 for now) -- 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: commits-unsubscr...@cloudstack.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
