Skip to content

Multiple changes to deploy scripts#11025

Merged
5 commits merged intomasterfrom
improve_deploy_scripts
Sep 3, 2018
Merged

Multiple changes to deploy scripts#11025
5 commits merged intomasterfrom
improve_deploy_scripts

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Sep 1, 2018

What does this PR do?

  1. Makes deploy_che.sh autonomous, ie the script can download templates without the need for a user to clone the entire repo. Once merged this script will be the default way to deploy Che to OpenShift in official documentation.
  2. Improves wait functions in deploy_che.sh. Using jq and parsing statuses isn't the right way to go. Instead, waiting for deployment replica is a foolproof way. It uses less code, and this approach is used in Ansible playbooks when waiting for a deployment
  3. Get rid of jq in deploy_che.sh since wait functions use -o=jsonpath rather than jq now, which is a true oc way.
  4. Get rid of jq in ocp.sh - the only function it was used in was the function that waited for ocp full boot. We do not use an internal docker registry in Origin at all during Che deployment. I'd say it's a legacy function since we indeed used s2i for Che multiuser (not anymore)
  5. Check if oc binary of the right version is present in PATH - downloading archives from GitHub takes time

@ghost ghost requested review from garagatyi, l0rd, riuvshin and sleshchenko as code owners September 1, 2018 07:52
@benoitf benoitf added the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Sep 1, 2018
@ghost ghost deleted a comment from riuvshin Sep 1, 2018
@riuvshin
Copy link
Copy Markdown
Contributor

riuvshin commented Sep 1, 2018

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

@ghost
Copy link
Copy Markdown
Author

ghost commented Sep 1, 2018

ci-test

@riuvshin
Copy link
Copy Markdown
Contributor

riuvshin commented Sep 1, 2018

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

@ghost
Copy link
Copy Markdown
Author

ghost commented Sep 1, 2018

ci-test

@riuvshin
Copy link
Copy Markdown
Contributor

riuvshin commented Sep 1, 2018

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

@ghost
Copy link
Copy Markdown
Author

ghost commented Sep 1, 2018

ci-test

@riuvshin
Copy link
Copy Markdown
Contributor

riuvshin commented Sep 2, 2018

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

@riuvshin
Copy link
Copy Markdown
Contributor

riuvshin commented Sep 2, 2018

ci-test

@riuvshin
Copy link
Copy Markdown
Contributor

riuvshin commented Sep 2, 2018

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

printInfo "Templates have been successfully saved to ${BASE_DIR}/templates"
fi
else printInfo "Templates directory found at ${BASE_DIR}/templates. Applying templates from this directory. Delete it to get the latest templates if necessary"
fi
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks like the formatting is a bit broken. Please consider reformatting:

getTemplates(){
  if [ ! -d "${BASE_DIR}/templates" ]; then
    printInfo "Local templates directory not found. Downloading templates..."
    curl -s https://codeload.github.com/eclipse/che/tar.gz/master | tar -xz --strip=3 che-master/deploy/openshift/templates -C ${BASE_DIR}
    OUT=$?
    if [ ${OUT} -eq 1 ]; then
      printError "Failed to curl and untar Eclipse Che repo because of an error"
      printInfo "You may need to manually clone or download content of https://github.com/eclipse/che/tree/master/deploy/openshift and re-run the script"
      exit ${OUT}
    else
      printInfo "Templates have been successfully saved to ${BASE_DIR}/templates"
    fi
  else
    printInfo "Templates directory found at ${BASE_DIR}/templates. Applying templates from this directory. Delete it to get the latest templates if necessary"
  fi
}

download_oc
else
# here we check is installed version is same version defined in script, if not we update version to one that defined in script.
if [[ $($OC_BINARY version 2> /dev/null | grep "oc v" | cut -d " " -f2 | cut -d '+' -f1 || true) != *"$OC_VERSION"* ]]; then
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Consider using tools for formatting.
The following is right here:

    if [[ $(oc version 2> /dev/null | grep "oc v" | cut -d " " -f2 | cut -d '+' -f1 || true) == *"$OC_VERSION"* ]]; then
        echo "Found oc ${OC_VERSION} in PATH. Using it"
        export OC_BINARY="oc"
    elif [[ ! -f $OC_BINARY ]]; then
        download_oc
    else
        # here we check is installed version is same version defined in script, if not we update version to one that defined in script.
        if [[ $($OC_BINARY version 2> /dev/null | grep "oc v" | cut -d " " -f2 | cut -d '+' -f1 || true) != *"$OC_VERSION"* ]]; then
            rm -f "$OC_BINARY" "$OCP_TOOLS_DIR"/README.md "$OCP_TOOLS_DIR"/LICENSE
            download_oc
        fi
    fi

printInfo "Local templates directory not found. Downloading templates..."
curl -s https://codeload.github.com/eclipse/che/tar.gz/master | tar -xz --strip=3 che-master/deploy/openshift/templates -C ${BASE_DIR}
OUT=$?
if [ ${OUT} -eq 1 ]; then
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It might be better to check for non zero return code as there are other error code other than 1

Copy link
Copy Markdown

@garagatyi garagatyi left a comment

Choose a reason for hiding this comment

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

LGTM

@ghost ghost merged commit 733d7bb into master Sep 3, 2018
@ghost ghost deleted the improve_deploy_scripts branch September 3, 2018 11:38
@benoitf benoitf added this to the 6.11.0 milestone Sep 3, 2018
@benoitf benoitf removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Sep 3, 2018
dmytro-ndp pushed a commit that referenced this pull request Sep 4, 2018
* Multiple changes to deploy scripts

* Remove debug echo

* Fix formatting

* Fix formatting

* Fixup
This pull request was closed.
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.

4 participants