Skip to content

hack,README: add quickstart-container script#20

Merged
openshift-merge-robot merged 2 commits into
openshift:masterfrom
steveej-forks:readme-add-docker-example
Jan 18, 2019
Merged

hack,README: add quickstart-container script#20
openshift-merge-robot merged 2 commits into
openshift:masterfrom
steveej-forks:readme-add-docker-example

Conversation

@steveej
Copy link
Copy Markdown
Contributor

@steveej steveej commented Dec 3, 2018 •
edited
Loading

@riuvshin PTAL

@openshift-ci-robot openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 3, 2018
@steveej steveej mentioned this pull request Dec 4, 2018
Merged
@steveej steveej force-pushed the readme-add-docker-example branch 3 times, most recently from 81557b9 to 39ae204 Compare December 4, 2018 19:22
@openshift-ci-robot openshift-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 4, 2018
@steveej steveej force-pushed the readme-add-docker-example branch 2 times, most recently from baeb829 to c55916a Compare December 4, 2018 19:46
@openshift-ci-robot openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Dec 4, 2018
@steveej steveej changed the title [WIP] README: add docker quickstart example Add a docker quickstart hack and mention in README Dec 4, 2018
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 4, 2018
@steveej steveej changed the title Add a docker quickstart hack and mention in README [WIP] Add a docker quickstart hack and mention in README Dec 4, 2018
@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 4, 2018
@steveej steveej force-pushed the readme-add-docker-example branch 2 times, most recently from 05f7689 to 63b5b51 Compare December 4, 2018 20:27
@steveej steveej mentioned this pull request Dec 4, 2018
Merged
@steveej steveej changed the title [WIP] Add a docker quickstart hack and mention in README [WIP] Add docker-quickstart hack and mention in README Dec 4, 2018
@steveej steveej force-pushed the readme-add-docker-example branch from 63b5b51 to 327686b Compare December 4, 2018 20:46
@steveej steveej changed the title [WIP] Add docker-quickstart hack and mention in README [WIP] Add docker-quickstart hack, mention in README, and enable on PR Ci runs Dec 4, 2018
@steveej steveej force-pushed the readme-add-docker-example branch from b8a5a67 to a1a38f6 Compare December 4, 2018 21:34
@steveej steveej changed the title [WIP] Add docker-quickstart hack, mention in README, and enable on PR Ci runs [WIP] Add docker-quickstart hack, mention in README, and enable on PR CI runs Dec 4, 2018
@steveej steveej changed the title [WIP] Add docker-quickstart hack, mention in README, and enable on PR CI runs Add docker-quickstart hack, mention in README, and enable on PR CI runs Dec 4, 2018
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 4, 2018
@steveej steveej force-pushed the readme-add-docker-example branch 2 times, most recently from 5731aa5 to 1c830de Compare December 5, 2018 14:56
@openshift-ci-robot openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 5, 2018
@openshift-ci-robot openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 7, 2018
@steveej
Copy link
Copy Markdown
Contributor Author

steveej commented Dec 12, 2018

@crawford PTAL

crawford
crawford reviewed Dec 12, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@crawford crawford left a comment

Choose a reason for hiding this comment

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

Who is the intended consumer of this script?

Comment thread README.md Outdated
Copy link
Copy Markdown
Contributor

@crawford crawford Dec 12, 2018

Choose a reason for hiding this comment

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

I don't want to point people at a personal repo.

Copy link
Copy Markdown
Contributor Author

@steveej steveej Dec 12, 2018

Choose a reason for hiding this comment

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

Me neither :-) We should have an alternative for it

Copy link
Copy Markdown
Contributor Author

@steveej steveej Dec 12, 2018

Choose a reason for hiding this comment

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

I was able to set up the test repositories under the redhat organization 🙌

Comment thread hack/quickstart-container.sh Outdated
@steveej
Copy link
Copy Markdown
Contributor Author

steveej commented Dec 12, 2018

Who is the intended consumer of this script?

I see it as a example towards we can point for a hands-on demonstration of how a cincinnati stack is set up. It's also useful for quick integration tests.

@steveej steveej force-pushed the readme-add-docker-example branch from 1dedea8 to 58d950b Compare December 12, 2018 22:30
@steveej
Copy link
Copy Markdown
Contributor Author

steveej commented Jan 8, 2019

@lucab what's your opinion on this? Do you see this being helpful?

@steveej
Copy link
Copy Markdown
Contributor Author

steveej commented Jan 8, 2019

/refresh
/retest

@steveej steveej force-pushed the readme-add-docker-example branch from 58d950b to 6b600ce Compare January 8, 2019 21:09
@openshift-ci-robot openshift-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 8, 2019
@steveej
Copy link
Copy Markdown
Contributor Author

steveej commented Jan 17, 2019

/cc @lucab

@lucab
Copy link
Copy Markdown
Contributor

lucab commented Jan 18, 2019 •
edited
Loading

@steveej sorry, I missed your earlier ping on this. For demo purpose, I think it makes sense to keep this at the documentation level. It is more helpful for people to see a quick example than having to dig through the variables in a bash script. My preference would be for something short and clean like https://github.com/coreos/matchbox/blob/master/Documentation/deployment.md#docker.

@steveej
Copy link
Copy Markdown
Contributor Author

steveej commented Jan 18, 2019

My preference would be for something short and clean like https://github.com/coreos/matchbox/blob/master/Documentation/deployment.md#docker.

I don't think it's a good comparison as that example has less complexity since it doesn't build the container first and also just instructs to run a single service. In comparison, the script I'm proposing here builds the container image, runs two services in containers and then uses curl to access the service stack. In short, this is a full life-cycle demo.

I'd prefer to have this script in the repository, even without a reference from the README. How about that?

@lucab
Copy link
Copy Markdown
Contributor

lucab commented Jan 18, 2019

I'd prefer to have this script in the repository, even without a reference from the README. How about that?

I am not in strong opposition here, so let's go for that. Please keep the other change in README (variables for test repo), as I think it's valuable there.

@steveej
hack: add quickstart-container script
15ad955 
This script locally builds a container image containing the
graph-builder and the policy-engine. It then runs both components in
separate containers and tests the policy-engine endpoint.
@steveej
README/quickstart: move magic strings to variables
82e1ae5 
The quickstart-container script enables to quickly set up and query a
containerized Cincinnati stack.
@steveej steveej force-pushed the readme-add-docker-example branch from 6b600ce to 82e1ae5 Compare January 18, 2019 15:16
@openshift-ci-robot openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 18, 2019
@steveej
Copy link
Copy Markdown
Contributor Author

steveej commented Jan 18, 2019

I am not in strong opposition here, so let's go for that. Please keep the other change in README (variables for test repo), as I think it's valuable there.

ACK and done.

@lucab
Copy link
Copy Markdown
Contributor

lucab commented Jan 18, 2019

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 18, 2019
@openshift-ci-robot
Copy link
Copy Markdown

openshift-ci-robot commented Jan 18, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lucab, riuvshin, steveeJ

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

The pull request process is described here

Details 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-merge-robot openshift-merge-robot merged commit b6b0e72 into openshift:master Jan 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lucab lucab Awaiting requested review from lucab

1 more reviewer

@crawford crawford crawford left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@lucab lucab

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@steveej @riuvshin @lucab @openshift-ci-robot @crawford @openshift-merge-robot