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

Switch gitserver to use openshift/origin-base #8902

Merged
merged 1 commit into from May 20, 2016

Conversation

csrwng
Copy link
Contributor

@csrwng csrwng commented May 17, 2016

Switches gitserver to use openshift/origin-base as its FROM image instead of openshift/origin. This prevents having 2 volume directives in the image, when only one is needed.

Also updated documentation to explain how to create a PVC for the git server so that it can work online.

Fixes BZ 1336318

@csrwng
Copy link
Contributor Author

csrwng commented May 17, 2016

@gabemontero ptal

@@ -38,6 +38,8 @@ Prerequisites:
$ oc policy add-role-to-user edit -z git
```

**NOTE:** To deploy the git server on OpenShift Online, you must switch to using persistent
storage. See the *Persistent Storage* section below.
Copy link
Contributor

Choose a reason for hiding this comment

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

Although I understand that https://github.com/openshift/origin/blob/master/examples/gitserver/gitserver.yaml is not a template like our other examples, would it still make sense to have an ephemeral and persistent volume version of this file for the gitserver.

If so, they couldn't the user just choose between the two versions like they do with our other examples?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bparees had suggested that. But there's a couple of reasons that I don't like it but maybe you can convince me otherwise:

  • The PVC claim size has to be adjusted to your environment's claim size, which is 1G for online, but doesn't have to be that for other environments.
  • The naming of things can get more complicated because now we'll have dc/git-ephemeral vs dc/git-persistent, etc. If possible I'd like it to remain short.

Copy link
Contributor

Choose a reason for hiding this comment

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

On Tue, May 17, 2016 at 11:17 AM, Cesar Wong notifications@github.com
wrote:

In examples/gitserver/README.md
#8902 (comment):

@@ -38,6 +38,8 @@ Prerequisites:
$ oc policy add-role-to-user edit -z git
```

+NOTE: To deploy the git server on OpenShift Online, you must switch to using persistent
+storage. See the Persistent Storage section below.

@bparees https://github.com/bparees had suggested that. But there's a
couple of reasons that I don't like it but maybe you can convince me
otherwise:

  • The PVC claim size has to be adjusted to your environment's claim
    size, which is 1G for online, but doesn't have to be that for other
    environments.

How is that different from our other templates? By direction from Dan we
ended up going with the online default of 1G for those.

  • The naming of things can get more complicated because now we'll have
    dc/git-ephemeral vs dc/git-persistent, etc. If possible I'd like it to
    remain short.

I think our users are used to the convention.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
https://github.com/openshift/origin/pull/8902/files/f7796cfc940a6305fef73e3c24b189c0856faff9#r63542087

Copy link
Contributor

Choose a reason for hiding this comment

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

Per feedback from @csrwng it looks like git truncated part of my last response via embedded email.

Reposting:

  1. Per direction from Dan we went with a 1 G PV default across all templates, so doing so here would be consistent.
  2. I think our users are already use to the ephemeral/persistent naming convention, so I don't think that should be concern

@gabemontero
Copy link
Contributor

One potentially out of the scope of this PR, but something to perhaps address long term, question ... otherwise IGTM.

Add ephemeral and persistent yaml files.
@csrwng
Copy link
Contributor Author

csrwng commented May 20, 2016

@gabemontero I added the persistent version of the yaml file and updated the README

@gabemontero
Copy link
Contributor

cool - thx .... igtm

@csrwng
Copy link
Contributor Author

csrwng commented May 20, 2016

[test]

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to 8d80e4b

@bparees
Copy link
Contributor

bparees commented May 20, 2016

lgtm [merge]

@openshift-bot
Copy link
Contributor

openshift-bot commented May 20, 2016

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_origin/5961/) (Image: devenv-rhel7_4251)

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to 8d80e4b

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/3952/)

@openshift-bot openshift-bot merged commit 58e612e into openshift:master May 20, 2016
@csrwng csrwng deleted the fix_gitserver_for_online branch July 19, 2016 15:34
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.

None yet

4 participants