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

Disable PVC creation for che-multiuser master #9236

Merged
merged 2 commits into from
Mar 28, 2018

Conversation

akorneta
Copy link
Contributor

What does this PR do?

Removes backup of che-home/lib folder.
Disables che-data PVC creation for che-muliuser deployed on Kubernetes/OpenShift.

The motivation for these changes is that we want to be able to run simultaneously two Che masters and more. To achieve that we need to avoid usage of shared PV between these instances, because in the cases when PVs does not support RWX mode we will get an issue with deploying of Che on different nodes.

What issues does this PR fix or reference?

#9040

@akorneta akorneta added status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. kind/task Internal things, technical debt, and to-do tasks to be performed. labels Mar 26, 2018
@akorneta akorneta self-assigned this Mar 26, 2018
@akorneta akorneta requested a review from benoitf as a code owner March 26, 2018 07:42
@akorneta akorneta requested a review from a user March 26, 2018 07:42
@akorneta akorneta requested a review from l0rd as a code owner March 26, 2018 07:42
@akorneta akorneta force-pushed the CHE-9040 branch 2 times, most recently from 96baae0 to d5526bd Compare March 26, 2018 08:03
Copy link
Member

@sleshchenko sleshchenko left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -292,6 +292,10 @@ CHE_CONFIG_FILE_PATH=${CHE_CONFIG_FILE_PATH:-${DEFAULT_CHE_CONFIG_FILE_PATH}}
cat "${CHE_DEPLOYMENT_FILE_PATH}" | \
sed "s/ image:.*/ image: \"${CHE_IMAGE_SANITIZED}\"/" | \
sed "s/ imagePullPolicy:.*/ imagePullPolicy: \"${IMAGE_PULL_POLICY}\"/" | \
if [[ "${CHE_MULTIUSER}" != "true" ]]; then
sed "s|#CHE_MASTER_PVC|- apiVersion: v1\n kind: PersistentVolumeClaim\n metadata:\n labels:\n app: che\n name: che-data-volume\n spec:\n accessModes:\n - ReadWriteOnce\n resources:\n requests:\n storage: 1Gi|" | \
Copy link
Member

@sleshchenko sleshchenko Mar 26, 2018

Choose a reason for hiding this comment

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

Is it possible to rewrite it in the following way?

CHE_MASTER_PVC="- apiVersion: v1\n
  kind: PersistentVolumeClaim\n
  metadata:\n
    labels:\n
      app: che\n
    name: che-data-volume\n
  spec:\n
    accessModes:\n
      - ReadWriteOnce\n
    resources:\n
      requests:\n
        storage: 1Gi"
...
cat "${CHE_DEPLOYMENT_FILE_PATH}" | \
    ...
    if [[ "${CHE_MULTIUSER}" != "true" ]]; then
    sed "s|#CHE_MASTER_PVC|${CHE_MASTER_PVC}|" | \
    sed "s|    #CHE_MASTER_VOLUME_MOUNTS.*|${CHE_MASTER_VOLUME_MOUNTS}|" | \
    sed "s|    #CHE_MASTER_VOLUMES.*|${CHE_MASTER_VOLUMES}|"; \
    else cat -; fi | \    
    inject_che_config "#CHE_MASTER_CONFIG" "${CHE_CONFIG_FILE_PATH}"
}

Copy link
Contributor

@riuvshin riuvshin left a comment

Choose a reason for hiding this comment

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

VERY GOOD

rm -rf ${CHE_DATA}/lib/*
mkdir -p ${CHE_DATA}/lib
cp -rf ${CHE_HOME}/lib/* "${CHE_DATA}"/lib
rm -rf ${CHE_DATA}/lib

Choose a reason for hiding this comment

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

Please, create an issue to remove this line after several releases. And update comment since it is out of date now.

@garagatyi
Copy link

@guydaichs @perspectivus1 please, review changes in helm deployment files

@akorneta
Copy link
Contributor Author

ci-test

@guydc
Copy link
Contributor

guydc commented Mar 26, 2018

Looks ok. @akorneta Can you test this on minikube?

@codenvy-ci
Copy link

ci-test build report:
Build details
Test report
selenium tests report data
docker image: eclipseche/che-server:9236
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

@akorneta
Copy link
Contributor Author

@guydaichs sure.

@akorneta akorneta merged commit 0afb356 into eclipse-che:master Mar 28, 2018
@akorneta akorneta deleted the CHE-9040 branch March 28, 2018 08:17
@akorneta akorneta removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Mar 28, 2018
@benoitf benoitf added this to the 6.4.0 milestone Mar 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/task Internal things, technical debt, and to-do tasks to be performed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants