SDN-5330: Keep rendering OVN IPsec when its daemonset not available - #2628
Conversation
pperiyasamy
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@pperiyasamy: This pull request references SDN-5330 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.19.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin repository. |
|
/retest |
|
/label acknowledge-critical-fixes-only |
openshift-ci
Bot
added
the
acknowledge-critical-fixes-only
|
@pperiyasamy: This pull request references SDN-5330 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.19.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin repository. |
|
@pperiyasamy: This pull request references SDN-5330 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.19.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin repository. |
|
/assign @anuragthehatter |
|
/hold this breaks ipsec disable, needs fixing |
openshift-ci
Bot
added
the
do-not-merge/hold
9be629c to
38fb790
Compare
|
/hold cancel |
openshift-ci
Bot
removed
the
do-not-merge/hold
|
/testwith openshift/cluster-network-operator/master/e2e-aws-ovn-ipsec-upgrade #2606 |
|
/test e2e-aws-ovn-ipsec-upgrade |
|
/retest |
This removes stale 4.13 IPsec upgrade handling code which is not a valid anymore for >=4.15 upgrade scenarios. Signed-off-by: Periyasamy Palanisamy <pepalani@redhat.com>
10fb24c to
90f7d4e
Compare
|
@pperiyasamy: This pull request references SDN-5330 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.19.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin repository. |
| renderIPsecDaemonSet = (isOVNIPsecActive && (isCNOIPsecMachineConfigPresent || isUserDefinedIPsecMachineConfigPresent || | ||
| isHypershiftHostedCluster)) || mode == operv1.IPsecModeFull |
There was a problem hiding this comment.
can you rather tweak isOVNIPsecActive above?
isOVNIPsecActiveOrRollingOut := bootstrapResult.OVN.IPsecUpdateStatus != nil && bootstrapResult.OVN.IPsecUpdateStatus.isOVNIPsecActiveOrRollingOut
// when OVN is rolling out, OVN IPsec might be fully or partially active or inactive. If MachineConfigs are not present, we know its inactive since we only stop rendering them once inactive.
isOVNIPsecActive := isOVNIPsecActiveOrRollingOut && (isCNOIPsecMachineConfigPresent || isUserDefinedIPsecMachineConfigPresent || isHypershiftHostedCluster)
The commit 4e57dcd is not complete because OVNIPsecStatus is still not set when none of the IPsec daemonset exists on the cluster at the time of machine config pools are updating (or) node is rebooted. Hence fixing it by OVNIPsecStatus is always set to reflect ipsec deployment state of the cluster and update the render pipeline to render ovn ipsec for the above mentioned scenarios. It renders ovn ipsec even before ipsec daemonsets are deployed when IPsec is freshly enabled on the cluster. That's ok because It will be effective only when the ovs-monitor-ipsec script is started and that's going to be done only when the ipsec pod is running. so we are safe to ignore it now. When IPsec is disabled from API, ovn ipsec is disabled followed by stop rendering ipsec machine config and ipsec daemonset. While ipsec machine configs are removed which would make ovnkube-node daemonset into progressing state and OVNIPsecActive condition becomes true again. Hence this commit considers machine config status as well so that the rendering pipeline will not render IPsec machine configs again. Signed-off-by: Periyasamy Palanisamy <pepalani@redhat.com>
90f7d4e to
ff0b147
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jcaamano, pperiyasamy 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 |
openshift-ci
Bot
added
the
approved
|
/retest-required |
|
/retest-required |
1 similar comment
|
@pperiyasamy: 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-sigs/prow repository. I understand the commands that are listed here. |
|
/retest-required |
|
[ART PR BUILD NOTIFIER] Distgit: cluster-network-operator |
6 participants
The commit 4e57dcd is not complete because
OVNIPsecStatusis still not set when none of the IPsec daemonset exists on the cluster at the time of machine config pools are updating(or) node is rebooted. Hence fixing it by
OVNIPsecStatusis always set to reflect ipsec deployment state of the clusterand update the render pipeline to render ovn ipsec for the above mentioned scenarios.
It renders ovn ipsec even before ipsec daemonsets are deployed when IPsec is freshly enabled on the cluster. That's ok because It will be effective only when the
ovs-monitor-ipsecscript is started and that's going to be done only when the ipsec pod is running. so we are safe to ignore it now.When IPsec is disabled from API, ovn ipsec is disabled followed by stop rendering ipsec machine config and ipsec daemonset. While ipsec machine configs are removed which would make
ovnkube-nodedaemonset into progressing state andOVNIPsecActivecondition becomes true again. Hence this pr considers machine configstatus as well so that the rendering pipeline will not render IPsec machine configs again.
It also removes dead code related to IPsec 4.13 upgrade which is no longer valid for upgrade scenarios beyond >= 4.15. This change is intended to be backported until 4.15 so still keeping legacy upgrade scenarios.