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.

Sign up for GitHub

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

Jump to bottom

Add gitfs and git_pillar fallback branch #51971

Merged
merged 8 commits into from
Jun 27, 2019
Merged

Add gitfs and git_pillar fallback branch #51971

merged 8 commits into from
Jun 27, 2019

Conversation

sathieu
Copy link
Contributor

@sathieu sathieu commented Mar 5, 2019 •
edited
Loading

What does this PR do?

Add a fallback branch when no matching branch is found

What issues does this PR fix or reference?

New Behavior

Example:

gitfs_remotes:
  - http://foo.com/states/top.git:
    - all_saltenvs: master
  - http://foo.com/states/apache.git:
    - fallback: master
  - http://foo.com/states/nginx.git:
    - fallback: master
  - http://foo.com/states/webapp.git:
    - fallback: develop

Where a state in webapp includes a state of apache.

With this PR, one can test a feature branch of webapp without needing to create the same branch in apache. Similarly, one can test a feature branch of apache without needing to create the same branch in webapp.

Tests written?

Yes

Commits signed with GPG?

Yes

@sathieu sathieu requested a review from a team as a code owner March 5, 2019 11:08
@hakanf
Copy link

hakanf commented Mar 8, 2019

Exactly what I need! +1 for this PR.

Thanks @sathieu

@sathieu
Copy link
Contributor Author

sathieu commented Mar 8, 2019

@hakanf If you're using GitPython, you can already test it. PyGit2 is not done yet

@hakanf
Copy link

hakanf commented Mar 9, 2019

@sathieu , yes I am using gitpython, and I have already begun to use the patch. It does indeed seem to work as expected...

@sathieu sathieu force-pushed the fallback branch 11 times, most recently from 2ebe0c7 to 30bd3be Compare March 15, 2019 16:16
@sathieu sathieu changed the title WIP: Add gitfs and git_pillar fallback branch Add gitfs and git_pillar fallback branch Mar 15, 2019
@sathieu
Copy link
Contributor Author

sathieu commented Mar 15, 2019

@terminalmage @dwoz, please review. I've added tests too.

@sathieu sathieu force-pushed the fallback branch 2 times, most recently from 89ba19b to 097f6c7 Compare March 20, 2019 20:25
waynew
waynew previously requested changes Mar 26, 2019
View reviewed changes
Copy link
Contributor

@waynew waynew left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! I found a couple of small typos in the docs portion.

doc/topics/tutorials/gitfs.rst Outdated Show resolved Hide resolved
doc/topics/tutorials/gitfs.rst Outdated Show resolved Hide resolved
@sathieu sathieu force-pushed the fallback branch 2 times, most recently from d233ff4 to f6fb380 Compare March 26, 2019 20:33
@sathieu
Copy link
Contributor Author

sathieu commented Mar 26, 2019

@waynew Comments addressed.

@waynew waynew dismissed their stale review March 27, 2019 17:31

Comments addressed

@sathieu
Copy link
Contributor Author

sathieu commented Apr 25, 2019

@waynew @dwoz Could you review and merge this one?

@sathieu
Copy link
Contributor Author

sathieu commented May 8, 2019

@waynew @dwoz @frogunder Any chance to have this in neon?
Thanks

@Ch3LL
Copy link
Contributor

Ch3LL commented Jun 21, 2019

thanks, i'll dive in. I'll try today, but most likely wont get to it till Monday. When i was troubleshooting I was using ubuntu18, so I'm guessing a different state is failing on centos7 during setup of the gitfs server.

@Ch3LL
Copy link
Contributor

Ch3LL commented Jun 24, 2019

I'm pretty sure i see the issue and working on a fix. might not have it ready till tomorrow.

@Ch3LL
Copy link
Contributor

Ch3LL commented Jun 25, 2019

alrighty lets try this again @sathieu sathieu#3

@Ch3LL Ch3LL mentioned this pull request Jun 25, 2019
Closed
@sathieu
Merge pull request #3 from Ch3LL/gitfs_virtualenv
9c3456c 
Set venv_bin for gitfs test setup if virtualenv exists
@sathieu
Copy link
Contributor Author

sathieu commented Jun 25, 2019

@Ch3LL thanks !

@waynew
Copy link
Contributor

waynew commented Jun 26, 2019

@Ch3LL looks like we've got one failure - https://jenkinsci.saltstack.com/job/pr-kitchen-centos7-py3/job/PR-51971/14/testReport/junit/integration.states.test_virtualenv_mod/VirtualenvTest/test_issue_1959_virtualenv_runas/

@sathieu Looks like we're out of date with neon here, would you like to rebase, or just merge neon into your branch?

@sathieu
Copy link
Contributor Author

sathieu commented Jun 26, 2019

@waynew I can do that, but keep in mind that both fixes done by @Ch3LL are needed for neon too. i.e. my PR doesn't introduce new bugs.

@sathieu sathieu mentioned this pull request Jun 27, 2019
Merged
@sathieu
Merge pull request #4 from saltstack/neon
072c9a2 
Merge Neon into fallback
@sathieu
Copy link
Contributor Author

sathieu commented Jun 27, 2019

@waynew :

@sathieu Looks like we're out of date with neon here, would you like to rebase, or just merge neon into your branch?

I just merged neon into my fallback branch.

@sathieu
Copy link
Contributor Author

sathieu commented Jun 27, 2019

@Ch3LL @waynew Everything is green now! Could you merge it?

@waynew waynew merged commit 9164b69 into saltstack:neon Jun 27, 2019
@waynew
Copy link
Contributor

waynew commented Jun 27, 2019

✨ :shipit: ✨

@sathieu
Copy link
Contributor Author

sathieu commented Jun 27, 2019

@waynew Thanks!

@sathieu sathieu deleted the fallback branch June 27, 2019 14:50
@waynew waynew mentioned this pull request Jan 9, 2020
Closed
@sathieu sathieu mentioned this pull request Jan 9, 2020
Open
@bdrung
Copy link
Contributor

bdrung commented Apr 3, 2020

Despite this merge request was merged into neon, it is neitehr part of the 3000 release or the master branch:

$ git log --oneline origin/master | grep "fallback branch"
$ git log --oneline origin/master | grep 51971
51971f8223 Add the frontend calls to support facter targets

@sathieu Can you please rebase your pull request against the master branch?

@sathieu
Copy link
Contributor Author

sathieu commented Apr 3, 2020

@bdrung It's listed on https://github.com/saltstack/salt/projects/5

@dwoz How to move forward on this?

@bdrung
Copy link
Contributor

bdrung commented Apr 3, 2020

@sathieu It's listed under "PR needs port to master". So I think that rebasing your patch set on top of master and creating another pull request would be a way to move forward.

bdrung added a commit to bdrung/salt that referenced this pull request Apr 15, 2020
@bdrung
Add gitfs and git_pillar fallback branch
1bcf426 
Add a fallback branch when no matching branch is found. Example:

```yaml
gitfs_remotes:
  - http://foo.com/states/top.git:
    - all_saltenvs: master
  - http://foo.com/states/apache.git:
    - fallback: master
  - http://foo.com/states/nginx.git:
    - fallback: master
  - http://foo.com/states/webapp.git:
    - fallback: develop
```

Where a state in `webapp` includes a state of `apache`.

With this change, one can test a feature branch of `webapp` without
needing to create the same branch in `apache`. Similarly, one can test a
feature branch of `apache` without needing to create the same branch in
`webapp`.

This is a forward port of pull request saltstack#51971 from Mathieu Parent.

Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com>
@bdrung bdrung mentioned this pull request Apr 15, 2020
Merged
3 tasks
@bdrung
Copy link
Contributor

bdrung commented Apr 15, 2020

I forward-ported this merge request in #56647, which took a lot of time due to a lot of merge conflicts due to blackening the code.

@sathieu
Copy link
Contributor Author

sathieu commented May 14, 2020

@waynew #56647 has been merged, please tag accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@waynew waynew waynew approved these changes

@terminalmage terminalmage terminalmage approved these changes

Assignees
No one assigned
Labels
Reviewers-Assigned ZRELEASED - Neon retired label
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@sathieu @hakanf @Ch3LL @waynew @bdrung @terminalmage