-
Notifications
You must be signed in to change notification settings - Fork 596
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
Add instructions on how to build dev environment #2472
Conversation
c2e0f6e
to
472684e
Compare
docs/developers-guide.md
Outdated
+--------------------------------------+--------------------------------------+----------------------------------+-------------+---------------------+------------------+----------+ | ||
|
||
# Call the LB | ||
$ curl 172.24.5.93 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I pulled the PR and tested locally it works fine
can this be updated as my env is different
+--------------------------------------+---------------------+------------------+--------------------------------------+--------------------------------------+----------------------------------+
| ID | Floating IP Address | Fixed IP Address | Port | Floating Network | Project |
+--------------------------------------+---------------------+------------------+--------------------------------------+--------------------------------------+----------------------------------+
| 15332df2-567d-411c-9ad0-6b28664a6fd9 | 172.24.5.101 | 10.1.0.20 | 36ecb62e-33c0-454d-bc08-1cbe6541874f | 74990c2b-fab0-4be1-9294-435a4b12f872 | 58b5f2a307cf4961b4d4918581c6fb38 |
+--------------------------------------+---------------------+------------------+--------------------------------------+--------------------------------------+----------------------------------+
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, sure.
@@ -32,7 +32,6 @@ cd "${REPO_ROOT}" || exit 1 | |||
# PULL_NUMBER and PULL_BASE_REF are Prow job environment variables | |||
PR_NUMBER="${PULL_NUMBER:-}" | |||
[[ -z $PR_NUMBER ]] && echo "PR_NUMBER is required" && exit 1 | |||
PR_BRANCH="${PULL_BASE_REF:-master}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume this used to be the branch that related to the submited PR
is it not necessary or you used other variable to replace?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I never found it used in the playbooks or script. Please correct me if I'm wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
um... I didn't check all code but from the naming convention I think we will do something like
master + PR => new test code thing to be tested on ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly I have no idea, but it does seem this is unused anywhere.
mdulko:cloud-provider-openstack/ (dev-environment) $ grep -R "github_pr_branch" .
mdulko:cloud-provider-openstack/ (dev-environment) $
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to make it safe, can you help change some code (e.g update a log in the code )
and after the CI run, we check the output and see whether log is really there to prove the PR is running on top of master, if it works then let's remove the log and get this PR merge?
Thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, thanks for the tip ,I think that make sense to me
This commit enhances the CI playbooks to allow deploying from `master` branch and adds documentation on how to use them to deploy a development environment.
472684e
to
24c6cd9
Compare
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jichenjc 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 |
@dulek I'll try this on my local VM and let you know if it works smoothly. |
@dulek works great |
/cherry-pick release-1.28 |
@kayrus: #2472 failed to apply on top of branch "release-1.28":
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Honestly I wouldn't bother too much with backporting this, it's just a development script. |
What this PR does / why we need it:
This commit enhances the CI playbooks to allow deploying from
master
branch and adds documentation on how to use them to deploy a development environment.Release note: