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.

Sign up for GitHub

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

Jump to bottom

Docker test container for travis login #3732

Merged
merged 1 commit into from
Aug 20, 2020
Merged

Docker test container for travis login #3732

merged 1 commit into from
Aug 20, 2020

Conversation

amitkrout
Copy link
Contributor

What type of PR is this?
/kind Feature

What does does this PR do / why we need it:
we are in process of getting the odo-bot GitHub token injected into prow. So creating a test container from where all these short of operation like travis cli install, travis login... etc will be launched

Which issue(s) this PR fixes:

Fixes : part of #2540

PR acceptance criteria:

Should not break master

How to test changes / Special notes to the reviewer:

$ docker run -it registry.svc.ci.openshift.org/openshift/release:golang-1.13
$ yum -y install sudo make wget gcc git httpd-tools ruby ruby-devel gcc-c++ patch \
    readline readline-devel zlib zlib-devel libffi-devel openssl-devel make bzip2 \
    autoconf automake libtool bison sqlite-devel
$ sudo curl -sSL https://rvm.io/mpapis.asc | gpg2 --import -
$ sleep 5
$ sudo curl -sSL https://rvm.io/pkuczynski.asc | gpg2 --import -
$ curl -L get.rvm.io | bash -s stable
$ source /etc/profile.d/rvm.sh
$ rvm reload
$ rvm requirements run
$ rvm install 2.7
$ gem install travis --pre --no-document

@openshift-ci-robot openshift-ci-robot added the kind/feature Categorizes issue as a feature request. For PRs, that means that the PR is the implementation label Aug 11, 2020
@codecov
Copy link

codecov bot commented Aug 11, 2020 •
edited
Loading

Codecov Report

Merging #3732 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3732   +/-   ##
=======================================
  Coverage   44.19%   44.19%           
=======================================
  Files         139      139           
  Lines       13388    13388           
=======================================
  Hits         5917     5917           
  Misses       6888     6888           
  Partials      583      583

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1e80be6...ad043fb. Read the comment docs.

prietyc123
prietyc123 suggested changes Aug 11, 2020
View reviewed changes
openshift-ci/build-root/Dockerfile.travislogin
@@ -0,0 +1,23 @@
# Dockerfile to bootstrap build and test in openshift-ci

FROM registry.svc.ci.openshift.org/openshift/release:golang-1.13
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of fetching image stream again, I would say let's implement it as https://github.com/openshift/odo/blob/master/openshift-ci/build-root/source-image/Dockerfile#L1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can not create a separate test container with out pulling the image. BTW the travis test container has certain steps to follow and i do not pollute the regular test container with the travis steps.

Ofcourse as per the requirement i may need https://github.com/openshift/odo/blob/master/openshift-ci/build-root/source-image/Dockerfile#L1 in my corresponding release job pr for copying the oc binary.

Copy link
Contributor

Choose a reason for hiding this comment

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

BTW the travis test container has certain steps to follow and i do not pollute the regular test container with the travis steps.

make sense 👍

But I would say once you get the overall workflow to do this, please ensure if we can have only one dockerfile for build. Other than that I am good with the changes.

/lgtm

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Corresponding ci pr created - openshift/release#10942

Copy link
Contributor Author

@amitkrout amitkrout Aug 12, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

But I would say once you get the overall workflow to do this, please ensure if we can have only one dockerfile for build.

@prietyc123 AFAIU It is not even possible because if i am installing packages in the source image for example https://github.com/openshift/odo/blob/master/openshift-ci/build-root/source-image/Dockerfile#L1 then it won't be propagated to the main build container. As per my requirement i do not need binary copy paste through the docker file, i need to install the packages for travis login in the main test container.

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Aug 12, 2020
@amitkrout amitkrout force-pushed the dockerTestContainerForTravisLogin branch from 16d0fb1 to ad043fb Compare August 20, 2020 05:31
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Aug 20, 2020
@prietyc123
Copy link
Contributor

New changes are detected. LGTM label has been removed.

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Aug 20, 2020
mohammedzee1000
mohammedzee1000 approved these changes Aug 20, 2020
View reviewed changes
Copy link
Contributor

@mohammedzee1000 mohammedzee1000 left a comment

Choose a reason for hiding this comment

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

looking good
/approve

@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mohammedzee1000

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. label Aug 20, 2020
@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

3 similar comments
@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@amitkrout
Copy link
Contributor Author

Please do not restart travis job. I am just experimenting the travis status is captured by prow bot or not

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

8 similar comments
@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

1 similar comment
@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

This was referenced Aug 20, 2020
Closed
Closed
@openshift-merge-robot openshift-merge-robot merged commit 0f4abad into redhat-developer:master Aug 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@prietyc123 prietyc123 prietyc123 requested changes

@mohammedzee1000 mohammedzee1000 mohammedzee1000 approved these changes

Assignees

@mohammedzee1000 mohammedzee1000

@prietyc123 prietyc123

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. kind/feature Categorizes issue as a feature request. For PRs, that means that the PR is the implementation lgtm Indicates that a PR is ready to be merged. Required by Prow.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@amitkrout @prietyc123 @openshift-ci-robot @openshift-bot @mohammedzee1000 @openshift-merge-robot