add vsphere ELB config #47676
add vsphere ELB config #47676
Conversation
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
/pj-rehearse periodic-ci-openshift-openshift-tests-private-release-4.15-amd64-nightly-vsphere-ipi-zones-multisubnets-external-lb-usermanaged-f28 |
openshift-merge-robot
added
the
needs-rebase
zhaozhanqi
force-pushed
the
elbvsphere
branch
from
b749719
to
9e353d0
Compare
openshift-merge-robot
removed
the
needs-rebase
zhaozhanqi
force-pushed
the
elbvsphere
branch
from
9e353d0
to
835d16c
Compare
|
/pj-rehearse periodic-ci-openshift-openshift-tests-private-release-4.16-amd64-nightly-vsphere-ipi-zones-multisubnets-external-lb-usermanaged-f28 |
1 similar comment
|
/pj-rehearse periodic-ci-openshift-openshift-tests-private-release-4.16-amd64-nightly-vsphere-ipi-zones-multisubnets-external-lb-usermanaged-f28 |
|
Issues in openshift/release go stale after 30d of inactivity. Mark the issue as fresh by commenting If this issue is safe to close now please do so with /lifecycle stale |
openshift-ci
bot
added
the
lifecycle/stale
openshift-merge-robot
added
the
needs-rebase
openshift-merge-robot
removed
the
needs-rebase
|
@zhaozhanqi, Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
@jianlinliu @liangxia Usually this Do you happen to know is there a way this can trigger a payload build from cluster-bot? to pre-merge testing this Pr openshift/api#1757 cc @sgaoshang |
Not sure if this is the one you want, https://docs.ci.openshift.org/docs/architecture/ci-operator/#testing-with-an-ephemeral-openshift-release |
openshift-merge-robot
added
the
needs-rebase
|
@zhaozhanqi, Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
openshift-merge-robot
removed
the
needs-rebase
|
/pj-rehearse periodic-ci-openshift-openshift-tests-private-release-4.16-amd64-nightly-vsphere-ipi-zones-multisubnets-external-lb-usermanaged-f28 |
|
/pj-rehearse periodic-ci-openshift-openshift-tests-private-release-4.16-amd64-nightly-vsphere-ipi-zones-multisubnets-external-lb-usermanaged-f28 |
1 similar comment
|
/pj-rehearse periodic-ci-openshift-openshift-tests-private-release-4.16-amd64-nightly-vsphere-ipi-zones-multisubnets-external-lb-usermanaged-f28 |
| @@ -99,6 +105,17 @@ networking: | |||
| machineNetwork: | |||
| - cidr: "${machine_cidr}" | |||
| EOF | |||
|
|
|||
| # if loadbalancer is UserManaged, it's mean using external LB, | |||
| # then keepalived and haproxy will not deployed, but coredns still keep | |||
There was a problem hiding this comment.
What is difference compared with the existing cucushift-installer-rehearse-vsphere-ipi-zones-multisubnets-external-lb install workflow ?
Does that mean if loadbalancer is OpenShiftManagedDefault, though we did not set apiVIPs and ingressVIPs in install-config.yaml, keepalived and haproxy are still deployed, right?
There was a problem hiding this comment.
I also talk to Dev for this question, see https://issues.redhat.com/browse/OPNET-305?focusedId=23430205&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-23430205
So like if using loadbalancer is UserManaged is IPI, if apiVIP and ingressVIP is empty, it's UPI
| @@ -2658,6 +2658,19 @@ tests: | |||
| test: | |||
| - chain: openshift-e2e-test-qe-destructive | |||
| workflow: cucushift-installer-rehearse-vsphere-ipi-zones-multisubnets-external-lb | |||
| - as: vsphere-ipi-zones-multisubnets-external-lb-usermanaged-f7 | |||
There was a problem hiding this comment.
f7 sounds a bit aggressive, vsphere does not have a big number of slice quota, I think we can set it to f28 to keep consistent with the above vsphere-ipi-zones-multisubnets-external-lb job.
And generally we always add a destructive ci job using the same install workflow as a pair.
| @@ -20,6 +20,12 @@ declare vsphere_extra_portgroup_2 | |||
| source "${SHARED_DIR}/vsphere_context.sh" | |||
| # shellcheck source=/dev/null | |||
| source "${SHARED_DIR}/govc.sh" | |||
| declare -a vips | |||
| mapfile -t vips < "${SHARED_DIR}"/vips.txt | |||
| APIVIP=${vips[0]} | |||
There was a problem hiding this comment.
I think here we can directly set APIVIP and INGRESSVIP to a correct value per different case:
declare -a vips
mapfile -t vips < "${SHARED_DIR}"/vips.txt
if [[ ${LB_TYPE} == "UserManaged" ]]; then
APIVIP=${vips[0]}
INGRESSVIP=${vips[1]}
else
APIVIP=""
INGRESSVIP=""
fi
After that, we can directly reference INGRESSVIP and APIVIP, and write then into "${CONFIG}" on line 38, and drop the if block on line 112.
There was a problem hiding this comment.
yeah, I was thinking it at the beginning. However if so we also need to set .platform.vsphere.loadBalancer.type which value should be UserManaged in this case for 4.16, not sure .platform.vsphere.loadBalancer.type will be supported in previous version. so I did not change the origin install-config.yaml. to avoid to cause the conflict. thus I don't need to regression test on other profile, WDYT?
There was a problem hiding this comment.
That should be not difficult.
declare -a vips
mapfile -t vips < "${SHARED_DIR}"/vips.txt
if [[ ${LB_TYPE} == "UserManaged" ]]; then
APIVIP=${vips[0]}
INGRESSVIP=${vips[1]}
LB_TYPE_DEF="loadBalancer:
type: UserManaged"
else
APIVIP=""
INGRESSVIP=""
LB_TYPE_DEF=
fi
cat >>"${CONFIG}" <<EOF
...
platform:
vsphere:
apiVIP: ${APIVIP}
ingressVIP: ${INGRESSVIP}
${LB_TYPE_DEF}
There was a problem hiding this comment.
apiVIPs also need to instead of apiVIP.
how about
if [ ${LB_TYPE} = "UserManaged" ]; then
yq --inplace eval-all 'select(fileIndex == 0) * select(fileIndex == 1)' "${CONFIG}" - <<< "
platform:
baremetal:
apiVIPs:
- ${vips[0]}
ingressVIPs:
- ${vips[1]}
loadBalancer:
type: UserManaged
"
fi
There was a problem hiding this comment.
Do you mean here it is updating platform.vsphere.apiVIPs, but not platform.vsphere.apiVIP? apiVIPs is newly introduced in this feature? If update platform.vsphere.apiVIPs, we still need to leave platform.vsphere.apiVIP to be empty? Do you know what is the difference between them?
Actually the initial purpose of the comment is not recommend to use yq, because it is download yq binary from internet, when multi jobs are running, it will hit network bottleneck issue.
There was a problem hiding this comment.
no, aipVIPs is not new, guess they already used for long time maybe from dualstack. apiVIP should be deprecated in future.
There was a problem hiding this comment.
https://github.com/openshift/installer/blob/master/pkg/asset/installconfig/vsphere/validation.go I did not see any validation for apiVIP..
There was a problem hiding this comment.
Sounds like we should drop the line platform.vsphere.apiVIP in install-config.yaml, but use the new one.
If using platform.vsphere.apiVIPs, we can do the same:
declare -a vips
mapfile -t vips < "${SHARED_DIR}"/vips.txt
if [[ ${LB_TYPE} == "UserManaged" ]]; then
APIVIPS_DEF="apiVIPs:
- ${vips[0]}"
INGRESSVIPS_DEF="ingressVIPs:
- ${vips[1]}"
LB_TYPE_DEF="loadBalancer:
type: UserManaged"
else
APIVIPS_DEF=""
INGRESSVIPS_DEF=""
LB_TYPE_DEF=""
fi
cat >>"${CONFIG}" <<EOF
...
platform:
vsphere:
${LB_TYPE_DEF}
${APIVIPS_DEF}
${INGRESSVIPS_DEF}
There was a problem hiding this comment.
ok, thanks. let me try
| @@ -2658,6 +2658,19 @@ tests: | |||
| test: | |||
| - chain: openshift-e2e-test-qe-destructive | |||
| workflow: cucushift-installer-rehearse-vsphere-ipi-zones-multisubnets-external-lb | |||
| - as: vsphere-ipi-zones-multisubnets-external-lb-usermanaged-f28 | |||
There was a problem hiding this comment.
generally we also need to add a destructive ci job using the same install workflow as a pair.
There was a problem hiding this comment.
@zhaozhanqi this is the last unresolved comment.
There was a problem hiding this comment.
oh, missed this one.
There was a problem hiding this comment.
@jianlinliu added and also update the title due to it's too long(> 62)
|
/pj-rehearse periodic-ci-openshift-openshift-tests-private-release-4.16-amd64-nightly-vsphere-ipi-zones-multisubnets-external-lb-usermanaged-f28 |
|
@zhaozhanqi: The following test failed, say
Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
|
/pj-rehearse periodic-ci-openshift-openshift-tests-private-release-4.14-amd64-nightly-vsphere-ipi-zones-multisubnets-external-lb-f28 |
|
/pj-rehearse periodic-ci-openshift-openshift-tests-private-release-4.16-amd64-nightly-vsphere-ipi-zones-multisubnets-external-lb-usermanaged-f28 |
@jianlinliu both this job and I triggered 4.16-elb with usermanagerd are passed at least in install cluster part |
|
/lgtm |
|
/pj-rehearse ack |
openshift-ci-robot
added
the
rehearsals-ack
|
cc @liangxia to review. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jianlinliu, liangxia, zhaozhanqi 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 |
openshift-ci
bot
added
the
approved
No description provided.