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

fetch port number of the running container and port on the host #87

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

mramanathan
Copy link
Contributor

Reference image used, https://hub.docker.com/r/tutum/hello-world/

This pipeline code snippet will (though Dockerfile is not supported which is two lines of code) fetch tutum/hello-world image, build it, start a new container based on it at port 80, will fetch host port that maps to the port of the running container, and run curl on the port of the localhost to check HTTP response.

@mramanathan
Copy link
Contributor Author

Not sure who reviews PRs in this GH repo. Could the concerned person evaluate this PR ?

stage('prep') {
steps {
script {
env.GIT_HASH = sh(

Choose a reason for hiding this comment

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

Is this GIT_HASH environment variable required by the script? I cannot see where it is dereferenced.

steps {
script {
env.GIT_HASH = sh(
script: "git show --oneline | head -1 | cut -d' ' -f1",
Copy link

@goostleek goostleek Apr 4, 2018

Choose a reason for hiding this comment

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

You can get the latest commit hash by a pure git command: git rev-parse HEAD. The result will be the same.

@mramanathan
Copy link
Contributor Author

@goostleek : Thanks for your time, I have cleaned 'prep' stage that gathers SHA-1 id of Git commit. Hope it's good to go now.

@goostleek
Copy link

goostleek commented Apr 5, 2018

@mramanathan Looks good to me. However, I'm not the maintainer of the repo. I'm just a casual wanderer that came across your PR because I starred the repo to look for the new examples as they appear :)

@mramanathan
Copy link
Contributor Author

@goostleek No probs, will wait for the maintainers' action.

@daniel-beck
Copy link
Member

There's a bunch of stuff here that's unnecessary for a Pipeline snippet (timestamps, cleanWs). That should probably be limited to jenkinsfile-examples and removed for simple-examples.

@daniel-beck
Copy link
Member

a076ca4 looks unrelated. I recommend you don't submit PRs from a master branch.

@mramanathan
Copy link
Contributor Author

@daniel-beck Agreed, I have trimmed the code and updated the PR with new commit, 704057f

@daniel-beck
Copy link
Member

daniel-beck commented Apr 9, 2018

@mramanathan All of that is still there: https:/jenkinsci/pipeline-examples/pull/87/files

Even if you delete the file it will clog up the history unless you rewrite Git history.

@mramanathan
Copy link
Contributor Author

@daniel-beck Hopefully it's better now, with rewritten Git history.

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.

3 participants