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

[TECH DEBT] support etcd api v3 #60325

Closed
6 of 7 tasks
jeff350 opened this issue Jun 7, 2021 · 5 comments · Fixed by #61911
Closed
6 of 7 tasks

[TECH DEBT] support etcd api v3 #60325

jeff350 opened this issue Jun 7, 2021 · 5 comments · Fixed by #61911
Assignees
Labels
effort-XL level of effort estimated in size Epic tech-debt
Milestone

Comments

@jeff350
Copy link

jeff350 commented Jun 7, 2021

  • - Investigate and decide which library (or libraries) should be used (activity, bug count, compatibility with current library)

Independent of the above task ensure that functional tests exist for

Once both functional tests exist and we have a selection for the library that we need to use, we can change the library imports, run the tests, and ensure that the appropriate behavior exists. If so, hooray! If not, we will need to update the modules above.

Is your feature request related to a problem? Please describe.

There are many features in salt such as using etcd as external pillar, returner, modules, or cache. the current implementation relies on the python-etcd package (https://github.com/jplana/python-etcd) that has not had a release since 2016 and does not support etcd API v3. Since etcd 3.4.0 etcd API v3 has become the default api, and etcd API v2 is disabled by default.

Before long salt will not work with etcd without downgrading to the etcd API v2. Moving forward it would be good for the salt team to look at deprecating the use of etcd API v2 and mover towards etcd API v3.

It looks like there are some possible alternatives for etcd packages that support etcd API v3.
https://github.com/kragniz/python-etcd3
https://github.com/Revolution1/etcd3-py

affected files
https://github.com/saltstack/salt/blob/master/salt/utils/etcd_util.py
https://github.com/saltstack/salt/blob/master/salt/cache/etcd_cache.py
https://github.com/saltstack/salt/blob/master/salt/modules/etcd_mod.py
https://github.com/saltstack/salt/blob/master/salt/states/etcd_mod.py
https://github.com/saltstack/salt/blob/master/salt/returners/etcd_return.py
https://github.com/saltstack/salt/blob/master/salt/pillar/etcd_pillar.py
https://github.com/saltstack/salt/blob/master/salt/sdb/etcd_db.py

Describe the solution you'd like
Salt should work with etcd with out requiring to downgrade to an old version of the API

related Issues
#55377
jplana/python-etcd#252

@jeff350 jeff350 added Feature new functionality including changes to functionality and code refactors, etc. needs-triage labels Jun 7, 2021
@sagetherage sagetherage added this to the Approved milestone Jun 8, 2021
@sagetherage
Copy link
Contributor

Right, this issue includes work to deprecate the use of etcd API v2 throughout salt and to add support for etcd api v3. Likely, this has work that needs to be done in phases. I will have this assigned and get details into the issue.

@sagetherage sagetherage added effort-XL level of effort estimated in size Epic labels Jun 8, 2021
@sagetherage sagetherage removed the Feature new functionality including changes to functionality and code refactors, etc. label Jun 15, 2021
@sagetherage sagetherage changed the title [FEATURE REQUEST] support etcd api v3 [TECH DEBT] support etcd api v3 Jun 15, 2021
@sagetherage sagetherage added the Silicon v3004.0 Release code name label Jun 15, 2021
@sagetherage sagetherage modified the milestones: Approved, Silicon Jun 15, 2021
@sagetherage
Copy link
Contributor

Wayne and I will plan out work in the Silicon release cycle and attempt to fix what is critical, first and not leave behind work, but that will need to be determined. Planning to break into smaller pieces of work at the end of June 2021 and more details to follow once we have had a chance to review.

@waynew
Copy link
Contributor

waynew commented Jun 26, 2021

@sagetherage I've taken a cursory glance at this:

  • It looks like we have a utils/etcd_util.py module (sidenote: this should be renamed in line with "name things once"), so hopefully we can simply make changes there
  • currently https://github.com/kragniz/python-etcd3 has failing builds, and does not look to have had a commit since May. Might be OK, or maybe we'd need to donate some efforts
  • currently https://github.com/Revolution1/etcd3-py hasn't seen a release since 2019 🤷

I'm sure going through the exercise of refactoring the Salt module would pop up several surprises...

Another thought is that we could produce extension module(s) for etcd support. Either one module that follows the factory pattern to decide which backend to use, or two modules that have the same module name/API but use different backends. Users would then choose which module to install.

@sagetherage sagetherage removed the Silicon v3004.0 Release code name label Jul 1, 2021
@sagetherage sagetherage modified the milestones: Silicon, Approved Jul 1, 2021
@sagetherage
Copy link
Contributor

@waynew let's create some issues around work and put those into Milestones. It is ok to have Epics span release cycles - in my mind - so I'm putting this into Approved as such until we write individual issues.

@MKLeb
Copy link
Contributor

MKLeb commented Mar 8, 2022

Looking more into candidate API v3 libraries. Here are my conclusions...

As far as compatibility with the current library, we are already using utils.etcd_util as a wrapper around the external etcd library, so small "incompatibilities" (naming conflicts, kwargs, etc...) should be relatively simple to circumvent. At most, I see us adding new functionality to the util, rather than completely refactoring its interface.

For the individual libraries themselves, @waynew's earlier comments still hold true, and not much has changed. For a more specific analysis...

A few follow-up questions...

  • The etcd API v2 seems to be no longer maintained. Should we phase it out altogether (with a deprecation, most likely)?
  • Since the candidate libraries seem to be maintained at a "slow cadence", is it worth implementing our own?

@MKLeb MKLeb self-assigned this Apr 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort-XL level of effort estimated in size Epic tech-debt
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants