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

Update on demand for the git cache on build server #262

Open
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

bartoszbetka
Copy link
Contributor

Resolves #191

@bartoszbetka bartoszbetka requested review from cameel and a user November 13, 2018 17:19
@bartoszbetka bartoszbetka force-pushed the feature-update-on-demand-for-the-git-cache-on-build-server branch 2 times, most recently from 0bf4005 to d920e29 Compare November 14, 2018 10:33
README.md Outdated Show resolved Hide resolved
service:
name: update-git-mirror
state: started
become_user: root
Copy link
Contributor

Choose a reason for hiding this comment

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

Please put it in a separate block. It makes it easier to see that you're switching users.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This require also add second block for task below.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes.

service:
name: update-git-mirror
state: started
become_user: root
Copy link
Contributor

Choose a reason for hiding this comment

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

You should not have to run it as root. The whole idea with setuid was to allow normal users to run it.

EDIT: After finishing the review I see that systemctl does not let use do what we wanted to achieve. We need to use sudo after all. See my comment in the issue that explains the problem and shows what we want to do instead: #191 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I know, we agreed to use systemctl with sudo. We only run sudo with systemctl that is less dangerous than when we run all scripts with sudo. The systemd service control what is execute.

Copy link
Contributor

Choose a reason for hiding this comment

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

I thought we agreed to use systemctl to avoid sudo (which is more problematic because it requires messing with /etc/sudoers/) and to use sudo as a last resort if systemd does not have any feature that let's you define who can run a service. It doesn't so let's go with sudo. I don't see any benefit in having a service if you need to use sudo anyway.

concent-builder/build-test-and-push.yml Outdated Show resolved Hide resolved
concent-builder/configure.yml Outdated Show resolved Hide resolved
concent-builder/consts.yml Outdated Show resolved Hide resolved
concent-builder/repositories.sh Show resolved Hide resolved
concent-builder/update-git-mirror.service Outdated Show resolved Hide resolved
concent-builder/update-git-mirror.service Outdated Show resolved Hide resolved
concent-builder/update-git-mirror.sh Outdated Show resolved Hide resolved
@bartoszbetka bartoszbetka force-pushed the feature-update-on-demand-for-the-git-cache-on-build-server branch from d920e29 to 67c2063 Compare November 15, 2018 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants