Add required dependency to support etcd3gw driver.

[khushboobhatia01]

• Added etcd3gw dependency required by tooz.
• Downgraded tenacity to 6.3.1 as tooz dependency on tenacity has always been < 7.0.0 and etcd3gw fails with higher versions. https://github.com/openstack/tooz/blob/174065f8750ff374e140fae956758b0b5ec2c473/requirements.txt#L9

[arm4b]

Thanks for the PR!

Per https://docs.stackstorm.com/coordination.html, we explicitly state that some backends might need a pip dependency which st2 doesn't provide and only redis is currently shipped by default.

Are you using etcd for coordination in your prod?

[khushboobhatia01]

Yes. We're using etcd right now, but we would want to enable service registry which requires etcd3gw.

[arm4b]

In the past, we thought to make it the default driver, but eventually, switched to Redis as recommended default which provided the best experience.
I've found #4608 and https://github.com/openstack/tooz/compare/master...StackStorm:etcd3_driver_fixes?expand=1 from the history and we experienced etcd3gw driver was broken in many ways, including limited functionality around the group membership which is required for service registry in st2.

@Kami if you remember more context about the etcd3gw.

@arms11 Are you folks using etcd as well as a st2 coordination backend or it's Redis?

[khushboobhatia01]

@armab etcd3 driver doesn't work with StackStorm because of https://review.opendev.org/c/openstack/tooz/+/466098/. I've confirmed this locally ST2 services keep crashing. However, I've verified etcd3gw driver and everything seems to work fine.

The groups created in etcd3gw are different than expected though.

We're looking to move to etcd3gw to use graceful shutdown implementation which is in progress (https://github.com/StackStorm/st2/pull/5428/files#diff-1ab1580c88e8cd9d04ca84d9f108163e136e32c9b50c7b829fbab8b70b4e6cffR155).

[arms11]

In the past, we thought to make it the default driver, but eventually, switched to Redis as recommended default which provided the best experience. I've found #4608 and https://github.com/openstack/tooz/compare/master...StackStorm:etcd3_driver_fixes?expand=1 from the history and we experienced etcd3gw driver was broken in many ways, including limited functionality around the group membership which is required for service registry in st2.

@Kami if you remember more context about the etcd3gw.

@arms11 Are you folks using etcd as well as a st2 coordination backend or it's Redis?

[arms11]

@khushboobhatia01 - I am sorry for the dumb click and closing this by mistake.
@armab To your question, we have been using Redis since I believe helm chart version 0.50. The last time I remember we found a bug in etcd driver related to keys api in StackStorm and older implementation was anyways not being maintained at which time we decided to move to Redis.

Just to understand, now since Orquesta workflow engine requires backend coordination and Redis is by default the chosen driver, does it make sense to go with etcd? Just curious.

[khushboobhatia01]

@khushboobhatia01 - I am sorry for the dumb click and closing this by mistake.

@armab To your question, we have been using Redis since I believe helm chart version 0.50. The last time I remember we found a bug in etcd driver related to keys api in StackStorm and older implementation was anyways not being maintained at which time we decided to move to Redis.

Just to understand, now since Orquesta workflow engine requires backend coordination and Redis is by default the chosen driver, does it make sense to go with etcd? Just curious.

@arms11 @armab We have been using etcd as coordination driver since we started using StackStorm about 3 years back. Since then we have stabilized our infrastructure to support a large number of executions. Currently we saw about 3M+ executions.

Our fleet and user base is going to increase more and I don't think we'll be moving to redis anytime soon.

[arm4b]

We're bundling Redis with the st2 core at this moment as a default coordination backend, which is shipped with all the official installation methods and e2e tested. We then recommend installing manually a pip dependency for other backends, per user needs.
https://docs.stackstorm.com/coordination.html

The problem of bundling multiple dependencies for backends is that there are many of them: zookeeper, consul, Memcached, etc, etc. Once we start including more than a single default one, it would be hard to stop from accepting other dependency backends, if proposed. It'll also increase the number of dependencies/package size/etc.

Let's collaborate on how to proceed here.

@StackStorm/tsc are we good with adding an additional coordination driver dependency in the st2 core?
That'll add additional etcd (apart from default Redis) as a pip dependency bundled in stackstorm. Though, Redis will remain officially e2e tested.

[cognifloyd]

With how we're creating the virtualenvs right now, I think this is the best approach. So 👍 from me

[nzlosh]

@armab bundling multiple tooz backend dependencies is a no-win situation. This PR demonstrates the situation by the fact that redis end to end testing works with tenacity 7.x, yet it breaks with the etcd3gw module. Since we only test the tooz redis backend, we can only guarantee production readiness specifically for this backend and no others.

Attempting to find common ground between 11 tooz backends and their python module dependencies is a road leading to dependency hell. This is especially true when factoring the module compatibility with a specific software versions multiplied by the number of different versions across all the St2 user base.

I'm normally pro-choice, in this particular case, I don't see how we can reasonably support bundling more than 1 backend. I also suggest we add a very strong warning in the coordinator documentation stating that only redis is officially supported for production use and the onus of testing/patching other backends rests completely with the user.

[amanda11]

I'm also concerned about having dependencies in that we don't test. If we find in future that tenacity > x is needed for redis, then we are going to have problems, as we might get into incompatabilities between version of packages needed for etcd3gw and redis.

[guzzijones]

I am a no on this as well

…

On Wed, Jan 12, 2022, 3:50 AM Carlos ***@***.***> wrote: @armab <https://github.com/armab> bundling multiple tooz backend dependencies is a no-win situation. This PR demonstrates the situation by the fact that redis end to end testing works with tenacity 7.x, yet it breaks with the etcd3gw module. Since we only test the tooz redis backend, we can only guarantee production readiness specifically for this backend and no others. Attempting to find common ground between 11 tooz backends and their python module dependencies is a road leading to dependency hell. This is especially true when factoring the module compatibility with a specific software versions multiplied by the number of different versions across all the St2 user base. I'm normally pro-choice, in this particular case, I don't see how we can reasonably support bundling more than 1 backend. I also suggest we add a very strong warning in the coordinator documentation stating that only redis is officially supported for production use and the onus of testing/patching other backends rests completely with the user. — Reply to this email directly, view it on GitHub <#5528 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACZ5TINJDACQYW33A7RVZ7LUVU6HBANCNFSM5LL6RL4A> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you are on a team that was mentioned.Message ID: ***@***.***>

[arm4b]

I hear that including other coordination backend dependencies like etcd3gw in st2 requirements.txt is a no-go at this moment.
Let's leave this open. Maybe we'll see more community requests in the future.

---

Speaking of tenacity which is already part of the st2 and is pulled by some other dependency (not Redis).

We have another bug report #5387, where Consul backend also fails with the new v7 tenacity version, similar to etcd3gw.

So considering we don't need to install any new packages, perhaps we could make it easier for the community and pin it?
If we'll find that some core st2 dependency needs a newer version, - we'll update it as the first priority without a notice or support promises. But as long as there is no conflict here and it doesn't break anything, - pinning should be fine.
Update: tenacity is a tooz dependency at the first place.

@StackStorm/maintainers Is that OK?

[guzzijones]

Wfm

…

On Wed, Jan 12, 2022, 6:00 PM Eugen Cusmaunsa ***@***.***> wrote: I hear that including other coordination backend dependencies like etcd3gw in st2 requirements.txt is a no-go at this moment. Let's leave this open. Maybe we'll see more community requests in the future. ------------------------------ Speaking of tenacity which is already part of the st2 and is pulled by some other dependency (not Redis <https://github.com/redis/redis-py/blob/master/requirements.txt>). We have another bug report #5387 <#5387>, where Consul backend also fails with the new v7 tenacity version, similar to etcd3gw. So considering we don't need to install any new packages, perhaps we could make it easier for the community and pin it? If we'll find that some core st2 dependency needs a newer version, - we'll update it as the first priority without a notice or support promises. But as long as there is no conflict here and it doesn't break anything, - pinning should be fine. Is that OK? — Reply to this email directly, view it on GitHub <#5528 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACZ5TIJP7CEQQFL6N6LPVB3UVYBYNANCNFSM5LL6RL4A> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you are on a team that was mentioned.Message ID: ***@***.***>

Prevent merging till there is consensus

[arm4b]

Also yeah, we've missed the point that tenacity is the tooz dependency in the first place, not specific to etcd3gw pip or any other backend.
https://github.com/openstack/tooz/blob/master/requirements.txt#L9

@khushboobhatia01 Could you please create a new dedicated PR for pinning the tenacity?
So we could at least merge that fix.

[khushboobhatia01]

Also yeah, we've missed the point that tenacity is the tooz dependency in the first place, not specific to etcd3gw pip or any other backend. https://github.com/openstack/tooz/blob/master/requirements.txt#L9

@khushboobhatia01 Could you please create a new dedicated PR for pinning the tenacity? So we could at least merge that fix.

Thanks @armab and others for the review. Will create another PR for pinning tenacity.

[arm4b]

Per discussion here, we'll need a PR that pins the tenacity dependency only.

[arm4b]

@khushboobhatia01 As a follow-up to this,
could you please create a PR to pin the tenacity dependency? I'd think it will be helpful for everyone.

[khushboobhatia01]

@khushboobhatia01 As a follow-up to this,

could you please create a PR to pin the tenacity dependency? I'd think it will be helpful for everyone.

Yes, @armab

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
2 out of 3 committers have signed the CLA.

✅ cognifloyd
✅ khushboobhatia01
❌ bkhushboo

---

bkhushboo seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}