Add Proxy CI for OpenStack platform#20800
Merged
openshift-merge-robot merged 1 commit intoopenshift:masterfrom Sep 7, 2021
Merged
Add Proxy CI for OpenStack platform#20800openshift-merge-robot merged 1 commit intoopenshift:masterfrom
openshift-merge-robot merged 1 commit intoopenshift:masterfrom
Conversation
MaysaMacedo
changed the title
openshift-ci
bot
added
the
do-not-merge/work-in-progress
MaysaMacedo
force-pushed
the
proxy-with-disconnected
branch
2 times, most recently
from
438ca5f to
44cbe1c
Compare
openshift-ci
bot
added
the
needs-rebase
MaysaMacedo
force-pushed
the
proxy-with-disconnected
branch
from
4e7b673 to
d2ffe14
Compare
openshift-ci
bot
removed
the
needs-rebase
MaysaMacedo
force-pushed
the
proxy-with-disconnected
branch
2 times, most recently
from
117c17a to
6653e6d
Compare
MaysaMacedo
force-pushed
the
proxy-with-disconnected
branch
4 times, most recently
from
5fd029b to
0805c03
Compare
EmilienM
reviewed
Aug 11, 2021
...ator/step-registry/openshift/e2e/openstack/proxy/openshift-e2e-openstack-proxy-workflow.yaml
Outdated
Show resolved
Hide resolved
EmilienM
reviewed
Aug 11, 2021
...erator/step-registry/openstack/conf/generateconfig/openstack-conf-generateconfig-commands.sh
Outdated
Show resolved
Hide resolved
EmilienM
reviewed
Aug 11, 2021
...registry/openstack/deprovision/machinesubnet/openstack-deprovision-machinesubnet-commands.sh
Outdated
Show resolved
Hide resolved
EmilienM
reviewed
Aug 11, 2021
.../step-registry/openstack/provision/bastionproxy/openstack-provision-bastionproxy-commands.sh
Outdated
Show resolved
Hide resolved
EmilienM
reviewed
Aug 11, 2021
.../step-registry/openstack/provision/bastionproxy/openstack-provision-bastionproxy-commands.sh
Outdated
Show resolved
Hide resolved
EmilienM
reviewed
Aug 11, 2021
.../step-registry/openstack/provision/bastionproxy/openstack-provision-bastionproxy-commands.sh
Outdated
Show resolved
Hide resolved
EmilienM
reviewed
Aug 11, 2021
.../step-registry/openstack/provision/bastionproxy/openstack-provision-bastionproxy-commands.sh
Outdated
Show resolved
Hide resolved
EmilienM
reviewed
Aug 11, 2021
.../step-registry/openstack/provision/bastionproxy/openstack-provision-bastionproxy-commands.sh
Outdated
Show resolved
Hide resolved
MaysaMacedo
force-pushed
the
proxy-with-disconnected
branch
5 times, most recently
from
eda90ec to
a3d6640
Compare
EmilienM
force-pushed
the
proxy-with-disconnected
branch
2 times, most recently
from
8268f41 to
3505c9c
Compare
MaysaMacedo
commented
Aug 25, 2021
ci-operator/config/openshift/release/openshift-release-master__nightly-4.9.yaml
Outdated
Show resolved
Hide resolved
Contributor
Author
|
/retest |
Member
|
/lgtm |
Contributor
Author
|
/retest |
andfasano
reviewed
Sep 1, 2021
Contributor
mandre
reviewed
Sep 1, 2021
Member
mandre
left a comment
mandre
left a comment
There was a problem hiding this comment.
A few nits, mostly about logging.
Member
There was a problem hiding this comment.
Just a small change to make it less confusing while debugging:
Suggested change
| echo "Skipping step due to CONFIG_TYPE being byon." | |
| echo "Skipping step due to CONFIG_TYPE being \"${CONFIG_TYPE}\"." |
Member
There was a problem hiding this comment.
Perhaps a comment would help understanding in which cases we need to remove the machines subnet from the router.
Member
There was a problem hiding this comment.
Suggested change
| >&2 echo "Created security group for ${CLUSTER_NAME}: ${sg_id}" | |
| >&2 echo "Created bastion security group for ${CLUSTER_NAME}: ${sg_id}" |
Member
There was a problem hiding this comment.
You've added DNS to the list, the log message is no longer current:
Suggested change
| >&2 echo "Security group rules created in ${sg_id} to allow SSH and squid access" | |
| >&2 echo "Created necessary security group rules in ${sg_id}" |
Member
There was a problem hiding this comment.
This assumes there will only ever be one interface for the server on the machines network. If we don't make it more robust, we should at least add a comment explaining our assumption.
Member
There was a problem hiding this comment.
I don't see why the bastion would have > 1 port connected on the machines network as of now, but you're right, this can evolve in the future; so we should document it here.
Comment on lines
64
to
65
Member
There was a problem hiding this comment.
Should we check that ${OPENSTACK_EXTERNAL_NETWORK} != ""?
Member
There was a problem hiding this comment.
I don't think we need, the previous command would have failed. OPENSTACK_EXTERNAL_NETWORK has a value for each platform, I think this is safe to keep it that way.
Member
There was a problem hiding this comment.
Suggested change
| echo "ZONES_COUNT was set to '${ZONES_COUNT}', although CONFIG_TYPE was not set to 'byon'." | |
| echo "ZONES_COUNT was set to '${ZONES_COUNT}', although CONFIG_TYPE was not set to 'byon' or 'proxy'." |
Member
|
/lgtm |
1 similar comment
Member
|
/lgtm |
Contributor
|
/approve |
Member
|
/retest |
5 similar comments
Contributor
Author
|
/retest |
Contributor
Author
|
/retest |
Contributor
Author
|
/retest |
Contributor
Author
|
/retest |
Contributor
Author
|
/retest |
Member
|
/lgtm |
Contributor
Author
|
/retest |
Contributor
Author
|
/assign @petr-muller |
petr-muller
reviewed
Sep 6, 2021
ci-operator/config/openshift/release/openshift-release-master__nightly-4.9.yaml
Outdated
Show resolved
Hide resolved
Contributor
Author
|
/retest |
This commit includes a new Job for a proxy installation with a restricted Network. The bastion VM is configured with two networks, one with access to the external world (Bastion network) and another without (Machines Network). The installer will use the restricted Machines network and rely on the Proxy for external access. The job will run on demand from the installer PRs and also every 72h in periodic. Co-Authored-By: Maysa Macedo <maysa.macedo95@gmail.com> Co-Authored-By: Emilien Macchi <emilien@redhat.com>
Member
|
/lgtm |
Contributor
Author
|
/assign stbenjam |
Contributor
Author
|
/retest |
Contributor
|
@MaysaMacedo: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. I understand the commands that are listed here. |
Member
|
/approve |
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: EmilienM, MaysaMacedo, patrickdillon, stbenjam The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Contributor
|
@MaysaMacedo: Updated the following 3 configmaps:
DetailsIn 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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This commit includes a new Job for a proxy
installation with a restricted Network. The
bastion VM is configured with two Networks,
one with access to the external world and
another without. The installer will use the
restricted Network and rely on the Proxy
for external access.
The job will run on demand from the installer PRs and also
every 72h in periodic.