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

this is going to fix lagoon issue #461 #463

Merged
merged 7 commits into from Jul 11, 2018

Conversation

Projects
None yet
6 participants
@markxtji
Contributor

markxtji commented Jun 15, 2018

this is going to fix this issue:

#461

@markxtji

This comment has been minimized.

Show comment
Hide comment
@markxtji

markxtji Jun 18, 2018

Contributor

@thom8 changed has been made according to what you suggested, build was successful but test failed on CI, which is quite hard for me to identify what exactly is the error.

Can you please advise what the possible error is ? The change I made is just a very simple change.

Contributor

markxtji commented Jun 18, 2018

@thom8 changed has been made according to what you suggested, build was successful but test failed on CI, which is quite hard for me to identify what exactly is the error.

Can you please advise what the possible error is ? The change I made is just a very simple change.

@thom8

This comment has been minimized.

Show comment
Hide comment
@thom8

thom8 Jun 18, 2018

Member

@markxtji can you try to replicate the issue locally? you'll have access to the build logs for debugging.

Member

thom8 commented Jun 18, 2018

@markxtji can you try to replicate the issue locally? you'll have access to the build logs for debugging.

@Schnitzel

I don't think this will have the requested result, see:

PERSISTENT_STORAGE_PATH=$(cat $DOCKER_COMPOSE_YAML | shyaml get-value services.$COMPOSE_SERVICE.labels.lagoon\\.persistent false)
if [ ! $PERSISTENT_STORAGE_PATH == "false" ]; then
TEMPLATE_PARAMETERS+=(-p PERSISTENT_STORAGE_PATH="${PERSISTENT_STORAGE_PATH}")
PERSISTENT_STORAGE_CLASS=$(cat $DOCKER_COMPOSE_YAML | shyaml get-value services.$COMPOSE_SERVICE.labels.lagoon\\.persistent\\.class false)
if [ ! $PERSISTENT_STORAGE_CLASS == "false" ]; then
TEMPLATE_PARAMETERS+=(-p PERSISTENT_STORAGE_CLASS="${PERSISTENT_STORAGE_CLASS}")
fi
PERSISTENT_STORAGE_NAME=$(cat $DOCKER_COMPOSE_YAML | shyaml get-value services.$COMPOSE_SERVICE.labels.lagoon\\.persistent\\.name false)
if [ ! $PERSISTENT_STORAGE_NAME == "false" ]; then
TEMPLATE_PARAMETERS+=(-p PERSISTENT_STORAGE_NAME="${PERSISTENT_STORAGE_NAME}")
fi
PERSISTENT_STORAGE_SIZE=$(cat $DOCKER_COMPOSE_YAML | shyaml get-value services.$COMPOSE_SERVICE.labels.lagoon\\.persistent\\.size false)
if [ ! $PERSISTENT_STORAGE_SIZE == "false" ]; then
TEMPLATE_PARAMETERS+=(-p PERSISTENT_STORAGE_SIZE="${PERSISTENT_STORAGE_SIZE}")
fi
fi

the PERSISTENT_STORAGE_SIZE and PERSISTENT_STORAGE_CLASS is only added when PERSISTENT_STORAGE_PATH is also set.

I'm happy though to move the check for PERSISTENT_STORAGE_SIZE and PERSISTENT_STORAGE_CLASS out of the wrapping if in order to allow the usecase here.

@markxtji

This comment has been minimized.

Show comment
Hide comment
@markxtji

markxtji Jun 22, 2018

Contributor

Hi @Schnitzel ,

Thank you for the information.
I reference from below file you created for ngninx-php-persistent, added PERSISTENT_STORAGE_PATH and changed the value of PERSISTENT_STORAGE_CLASS to be empty.

https://github.com/amazeeio/lagoon/blob/master/images/oc-build-deploy-dind/openshift-templates/nginx-php-persistent/pvc.yml

When I use it from dockercompose file, I should use below lables, am I correct ?

labels:
  lagoon.type: elasticsearch
  lagoon.persistent: /usr/share/elasticsearch/data
  lagoon.persistent.class: slow
  lagoon.name: elasticsearch

where can I find the mapping between the lables and parameters in the pvc.yml ?

Thank you.

Contributor

markxtji commented Jun 22, 2018

Hi @Schnitzel ,

Thank you for the information.
I reference from below file you created for ngninx-php-persistent, added PERSISTENT_STORAGE_PATH and changed the value of PERSISTENT_STORAGE_CLASS to be empty.

https://github.com/amazeeio/lagoon/blob/master/images/oc-build-deploy-dind/openshift-templates/nginx-php-persistent/pvc.yml

When I use it from dockercompose file, I should use below lables, am I correct ?

labels:
  lagoon.type: elasticsearch
  lagoon.persistent: /usr/share/elasticsearch/data
  lagoon.persistent.class: slow
  lagoon.name: elasticsearch

where can I find the mapping between the lables and parameters in the pvc.yml ?

Thank you.

@Schnitzel

This comment has been minimized.

Show comment
Hide comment
@Schnitzel

Schnitzel Jun 22, 2018

Member

@markxtji
Hi!

So there are two systems in place here:
For the ngninx-php-persistent we actually want to give the developers the possibility to define the Path of the persistent storage, as this depends on the application they are deploying (like Drupal needs /app/web/sites/default/files, while Wordpress needs something like /app/files). So allowing the user to define the path is a good thing.

On the other side, for elasticsearch we DON'T want the developers to define the path, as elasticsearch expects the path of the persistent storage always to be at /usr/share/elasticsearch/data. If a developer defines another path (let's say /foo/data) then that persistent storage might be mounted, but elasticsearch will still put it's data at /usr/share/elasticsearch/data and with that create a lot of confusion.

So that's why I am suggesting to move the adding of the parameters here:

PERSISTENT_STORAGE_PATH=$(cat $DOCKER_COMPOSE_YAML | shyaml get-value services.$COMPOSE_SERVICE.labels.lagoon\\.persistent false)
if [ ! $PERSISTENT_STORAGE_PATH == "false" ]; then
TEMPLATE_PARAMETERS+=(-p PERSISTENT_STORAGE_PATH="${PERSISTENT_STORAGE_PATH}")
PERSISTENT_STORAGE_CLASS=$(cat $DOCKER_COMPOSE_YAML | shyaml get-value services.$COMPOSE_SERVICE.labels.lagoon\\.persistent\\.class false)
if [ ! $PERSISTENT_STORAGE_CLASS == "false" ]; then
TEMPLATE_PARAMETERS+=(-p PERSISTENT_STORAGE_CLASS="${PERSISTENT_STORAGE_CLASS}")
fi
PERSISTENT_STORAGE_NAME=$(cat $DOCKER_COMPOSE_YAML | shyaml get-value services.$COMPOSE_SERVICE.labels.lagoon\\.persistent\\.name false)
if [ ! $PERSISTENT_STORAGE_NAME == "false" ]; then
TEMPLATE_PARAMETERS+=(-p PERSISTENT_STORAGE_NAME="${PERSISTENT_STORAGE_NAME}")
fi
PERSISTENT_STORAGE_SIZE=$(cat $DOCKER_COMPOSE_YAML | shyaml get-value services.$COMPOSE_SERVICE.labels.lagoon\\.persistent\\.size false)
if [ ! $PERSISTENT_STORAGE_SIZE == "false" ]; then
TEMPLATE_PARAMETERS+=(-p PERSISTENT_STORAGE_SIZE="${PERSISTENT_STORAGE_SIZE}")
fi
fi

outside of the if [ ! $PERSISTENT_STORAGE_PATH == "false" ]; check in order to have the possibility to define just that in the docker-compose.yaml:

labels:
  lagoon.type: elasticsearch
  lagoon.persistent.class: slow

makes sense? Happy to do that part in build-deploy-docker-compose.sh if it's too confusing :)

where can I find the mapping between the lables and parameters in the pvc.yml ?

that also happens here:

PERSISTENT_STORAGE_PATH=$(cat $DOCKER_COMPOSE_YAML | shyaml get-value services.$COMPOSE_SERVICE.labels.lagoon\\.persistent false)
if [ ! $PERSISTENT_STORAGE_PATH == "false" ]; then
TEMPLATE_PARAMETERS+=(-p PERSISTENT_STORAGE_PATH="${PERSISTENT_STORAGE_PATH}")
PERSISTENT_STORAGE_CLASS=$(cat $DOCKER_COMPOSE_YAML | shyaml get-value services.$COMPOSE_SERVICE.labels.lagoon\\.persistent\\.class false)
if [ ! $PERSISTENT_STORAGE_CLASS == "false" ]; then
TEMPLATE_PARAMETERS+=(-p PERSISTENT_STORAGE_CLASS="${PERSISTENT_STORAGE_CLASS}")
fi
PERSISTENT_STORAGE_NAME=$(cat $DOCKER_COMPOSE_YAML | shyaml get-value services.$COMPOSE_SERVICE.labels.lagoon\\.persistent\\.name false)
if [ ! $PERSISTENT_STORAGE_NAME == "false" ]; then
TEMPLATE_PARAMETERS+=(-p PERSISTENT_STORAGE_NAME="${PERSISTENT_STORAGE_NAME}")
fi
PERSISTENT_STORAGE_SIZE=$(cat $DOCKER_COMPOSE_YAML | shyaml get-value services.$COMPOSE_SERVICE.labels.lagoon\\.persistent\\.size false)
if [ ! $PERSISTENT_STORAGE_SIZE == "false" ]; then
TEMPLATE_PARAMETERS+=(-p PERSISTENT_STORAGE_SIZE="${PERSISTENT_STORAGE_SIZE}")
fi
fi

Member

Schnitzel commented Jun 22, 2018

@markxtji
Hi!

So there are two systems in place here:
For the ngninx-php-persistent we actually want to give the developers the possibility to define the Path of the persistent storage, as this depends on the application they are deploying (like Drupal needs /app/web/sites/default/files, while Wordpress needs something like /app/files). So allowing the user to define the path is a good thing.

On the other side, for elasticsearch we DON'T want the developers to define the path, as elasticsearch expects the path of the persistent storage always to be at /usr/share/elasticsearch/data. If a developer defines another path (let's say /foo/data) then that persistent storage might be mounted, but elasticsearch will still put it's data at /usr/share/elasticsearch/data and with that create a lot of confusion.

So that's why I am suggesting to move the adding of the parameters here:

PERSISTENT_STORAGE_PATH=$(cat $DOCKER_COMPOSE_YAML | shyaml get-value services.$COMPOSE_SERVICE.labels.lagoon\\.persistent false)
if [ ! $PERSISTENT_STORAGE_PATH == "false" ]; then
TEMPLATE_PARAMETERS+=(-p PERSISTENT_STORAGE_PATH="${PERSISTENT_STORAGE_PATH}")
PERSISTENT_STORAGE_CLASS=$(cat $DOCKER_COMPOSE_YAML | shyaml get-value services.$COMPOSE_SERVICE.labels.lagoon\\.persistent\\.class false)
if [ ! $PERSISTENT_STORAGE_CLASS == "false" ]; then
TEMPLATE_PARAMETERS+=(-p PERSISTENT_STORAGE_CLASS="${PERSISTENT_STORAGE_CLASS}")
fi
PERSISTENT_STORAGE_NAME=$(cat $DOCKER_COMPOSE_YAML | shyaml get-value services.$COMPOSE_SERVICE.labels.lagoon\\.persistent\\.name false)
if [ ! $PERSISTENT_STORAGE_NAME == "false" ]; then
TEMPLATE_PARAMETERS+=(-p PERSISTENT_STORAGE_NAME="${PERSISTENT_STORAGE_NAME}")
fi
PERSISTENT_STORAGE_SIZE=$(cat $DOCKER_COMPOSE_YAML | shyaml get-value services.$COMPOSE_SERVICE.labels.lagoon\\.persistent\\.size false)
if [ ! $PERSISTENT_STORAGE_SIZE == "false" ]; then
TEMPLATE_PARAMETERS+=(-p PERSISTENT_STORAGE_SIZE="${PERSISTENT_STORAGE_SIZE}")
fi
fi

outside of the if [ ! $PERSISTENT_STORAGE_PATH == "false" ]; check in order to have the possibility to define just that in the docker-compose.yaml:

labels:
  lagoon.type: elasticsearch
  lagoon.persistent.class: slow

makes sense? Happy to do that part in build-deploy-docker-compose.sh if it's too confusing :)

where can I find the mapping between the lables and parameters in the pvc.yml ?

that also happens here:

PERSISTENT_STORAGE_PATH=$(cat $DOCKER_COMPOSE_YAML | shyaml get-value services.$COMPOSE_SERVICE.labels.lagoon\\.persistent false)
if [ ! $PERSISTENT_STORAGE_PATH == "false" ]; then
TEMPLATE_PARAMETERS+=(-p PERSISTENT_STORAGE_PATH="${PERSISTENT_STORAGE_PATH}")
PERSISTENT_STORAGE_CLASS=$(cat $DOCKER_COMPOSE_YAML | shyaml get-value services.$COMPOSE_SERVICE.labels.lagoon\\.persistent\\.class false)
if [ ! $PERSISTENT_STORAGE_CLASS == "false" ]; then
TEMPLATE_PARAMETERS+=(-p PERSISTENT_STORAGE_CLASS="${PERSISTENT_STORAGE_CLASS}")
fi
PERSISTENT_STORAGE_NAME=$(cat $DOCKER_COMPOSE_YAML | shyaml get-value services.$COMPOSE_SERVICE.labels.lagoon\\.persistent\\.name false)
if [ ! $PERSISTENT_STORAGE_NAME == "false" ]; then
TEMPLATE_PARAMETERS+=(-p PERSISTENT_STORAGE_NAME="${PERSISTENT_STORAGE_NAME}")
fi
PERSISTENT_STORAGE_SIZE=$(cat $DOCKER_COMPOSE_YAML | shyaml get-value services.$COMPOSE_SERVICE.labels.lagoon\\.persistent\\.size false)
if [ ! $PERSISTENT_STORAGE_SIZE == "false" ]; then
TEMPLATE_PARAMETERS+=(-p PERSISTENT_STORAGE_SIZE="${PERSISTENT_STORAGE_SIZE}")
fi
fi

@markxtji

This comment has been minimized.

Show comment
Hide comment
@markxtji

markxtji Jun 22, 2018

Contributor

HI @Schnitzel ,
thank you for the detail information.
I have updated according to what you suggested, please review again.
thank you.

Mark

Contributor

markxtji commented Jun 22, 2018

HI @Schnitzel ,
thank you for the detail information.
I have updated according to what you suggested, please review again.
thank you.

Mark

was fixed with my review

@twardnw twardnw referenced this pull request Jul 9, 2018

Merged

add volume size for mariadb #487

Schnitzel added a commit that referenced this pull request Jul 10, 2018

@Schnitzel Schnitzel merged commit 2ff63e9 into amazeeio:master Jul 11, 2018

1 check failed

continuous-integration/jenkins/pr-merge This commit cannot be built
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment