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

Rework importing of os templates #49

Merged
merged 7 commits into from
Jun 6, 2018

Conversation

arilivigni
Copy link
Member

  • Always load helper containers {linchpin,ansible}-executor
    • project_repo != sample_project_repo
  • Reworked to reduce code duplication
  • project, helper, jenkins sample, samples if needed

Ari LiVigni added 3 commits June 4, 2018 17:01
* Always load helper containers {linchpin,ansible}-executor
  * project_repo != sample_project_repo
* Always load helper containers {linchpin,ansible}-executor
  * project_repo != sample_project_repo
* Reworked to reduce code duplication
* Always load helper containers {linchpin,ansible}-executor
  * project_repo != sample_project_repo
* Reworked to reduce code duplication
@arilivigni arilivigni requested a review from scoheb June 5, 2018 01:27
@arilivigni
Copy link
Member Author

@A890571


# Check if Jenkins is running
- name: "Check to see if a Jenkins Master instance is running"
shell: "{{ oc_bin }} get pods | grep -i 'running' | grep -i 'jenkins' | tail -1 | awk '{print $1}'"
Copy link
Contributor

Choose a reason for hiding this comment

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

are we in the correct project here for the query?

Copy link
Member Author

Choose a reason for hiding this comment

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

In my testing I was. Is there something I should do at the top like oc project {{ openshift_project }}

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess a shell task right before would be a safe bet.

Copy link
Member Author

Choose a reason for hiding this comment

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

+1 - Sure I will ad that in the next change.

* Always load helper containers {linchpin,ansible}-executor
  * project_repo != sample_project_repo
* Reworked to reduce code duplication
* Added switch to project shell command before making changes to the current project
# Switch to right OpenShift project defined in {{ {{ openshift_project }}" }}
- name: "Switch to project {{ openshift_project }}"
shell: "{{ oc_bin }} project {{ openshift_project }}"
register: cluster_ip_register
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure we need these 2 lines...

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean you can remove register and when

Ari LiVigni added 2 commits June 5, 2018 15:14
* Always load helper containers {linchpin,ansible}-executor
  * project_repo != sample_project_repo
* Reworked to reduce code duplication
* Added switch to project shell command before making changes to the current project
  * fixed issue - remvoed when and register
* Always load helper containers {linchpin,ansible}-executor
  * project_repo != sample_project_repo
* Reworked to reduce code duplication
* Remove switch to project we already do that
@arilivigni
Copy link
Member Author

@scoheb Can you review again after the last changes. I verified when deleting everything and running the setup CVP and Helper containers were added as expected

@@ -13,7 +13,7 @@

# Get ip of the cluster unless it was provided
- name: "Get the cluster Server URL for project {{ openshift_project }}"
shell: "{{ oc_bin }} status | egrep '{{ openshift_project }}.*server' | awk '{print $NF}'"
shell: "{{ oc_bin }} status | grep 'https' | awk '{print $NF}'"
Copy link
Contributor

@scoheb scoheb Jun 6, 2018

Choose a reason for hiding this comment

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

probably should have a | head -1 after the awk

my output was:

shebert@localhost~/work/cvp/pipeline:fix-brew-nvr-scope$ oc status | grep https | awk '{print $NF}'
https://127.0.0.1:8443
(svc/jenkins)
docker.io/openshift/jenkins-2-centos7:v3.9
docker.io/openshift/jenkins-2-centos7:v3.9
https://gitlab.cee.redhat.com/Feuerkasten/xCI-systems.git#stage

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks and done

* Always load helper containers {linchpin,ansible}-executor
  * project_repo != sample_project_repo
* Reworked to reduce code duplication
* Remove switch to project we already do that
* Narrow match with head -1
@arilivigni arilivigni merged commit f34baca into CentOS-PaaS-SIG:master Jun 6, 2018
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