OCPBUGS-32525: Check every MachineConfigPool for IPsec plugin existence - #2349
Merged
openshift-merge-bot[bot] merged 1 commit intoMay 6, 2024
Merged
Conversation
openshift-ci-robot
added
jira/valid-reference
Contributor
|
@pperiyasamy: This pull request references Jira Issue OCPBUGS-32525, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: The bug has been updated to refer to the pull request using the external bug tracker. 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. |
Member
Author
Contributor
|
code looks fine to me, but I'm not the expert here. |
Member
Author
|
/retest |
pperiyasamy
force-pushed
the
ipsec-mcp-pools
branch
from
April 26, 2024 12:08
eb39d4a to
8af566e
Compare
jcaamano
reviewed
Apr 30, 2024
Comment on lines
+29
to
+34
| var ( | ||
| MasterRoleMachineConfigLabel = "machineconfiguration.openshift.io/role=master" | ||
| WorkerRoleMachineConfigLabel = "machineconfiguration.openshift.io/role=worker" | ||
| ) | ||
|
|
Contributor
There was a problem hiding this comment.
nit: make this const and unexport
| } | ||
| var mcpStatuses []mcfgv1.MachineConfigPoolStatus | ||
| for _, mcp := range mcpList.Items { | ||
| for lKey, lValue := range mcp.Spec.MachineConfigSelector.MatchLabels { |
Contributor
There was a problem hiding this comment.
What if MatchExpressions is used instead of MatchLabels?
machineConfigSelector:
matchExpressions:
- {key: machineconfiguration.openshift.io/role, operator: In, values: [worker,infra]}
from https://github.com/openshift/machine-config-operator/blob/master/docs/custom-pools.md
I guess we need to use some kind of kubernetes library for label matching here.
Member
Author
There was a problem hiding this comment.
right! this is what infra and workload pools uses for machineConfigSelector, fixed it now.
pperiyasamy
force-pushed
the
ipsec-mcp-pools
branch
from
April 30, 2024 12:03
8af566e to
28f2af8
Compare
Member
Author
|
/test control-plane-ipsec-24nodes |
Contributor
|
@pperiyasamy: The specified target(s) for
The following commands are available to trigger optional jobs:
Use
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. |
Member
Author
|
/retest |
Member
Author
|
/retest-required |
Member
Author
|
/retest |
Member
Author
|
/test e2e-ovn-ipsec-step-registry |
While testing IPsec upgrade on 24 node cluster in CI, it's noticed there are more than two (master, worker) machine config pools present. For example infra and workload pools were present which belong worker role. Hence this commit queries all machine config pools based on master and worker roles and check ipsec plugin existence on every pool. Signed-off-by: Periyasamy Palanisamy <pepalani@redhat.com>
pperiyasamy
force-pushed
the
ipsec-mcp-pools
branch
from
May 6, 2024 07:34
28f2af8 to
6aa2f0f
Compare
Member
Author
|
/retest |
Member
Author
|
/test ? |
Contributor
|
@pperiyasamy: The following commands are available to trigger required jobs:
The following commands are available to trigger optional jobs:
Use
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. |
Member
Author
|
/test e2e-aws-ovn-ipsec-upgrade |
Contributor
|
/lgtm |
Contributor
|
/cherry-pick release-4.15 |
|
@yuvalk: #2349 failed to apply on top of branch "release-4.15": 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-sigs/prow repository. |
This was referenced Jul 17, 2024
pperiyasamy
added a commit
to pperiyasamy/cluster-network-operator
that referenced
this pull request
Jan 20, 2025
Due to known reasons as introduced in the PR openshift#2349, The IPsec daemonset is removed at the time of OCP upgrade or MCP in progressing state. But it also accidently disabling IPsec in OVN. This causes ovs-monitor-ipsec to refresh existing ipsec connections unnecessarily (when pod comes up) as it is not able to find remote_name from tunnel. This may also trigger deleting IPsec connection entries from openshift.conf file if ovs-monitor-ipsec is not killed timely when ipsec daemonset is removed. So this commit enables ovn ipsec option as long as the API is set with Full mode. Signed-off-by: Periyasamy Palanisamy <pepalani@redhat.com>
pperiyasamy
added a commit
to pperiyasamy/cluster-network-operator
that referenced
this pull request
Jan 22, 2025
The following change on the machine pool `status.MachineCount == status.UpdatedMachineCount && hasSourceInMachineConfigStatus(status, machineConfigs)` is introduced with PR openshift#2349 which ensures IPsec machine config is always installed on all the nodes in the cluster, So this is deleting the IPsec daemonset as per the CNO state machine for IPsec when the condition is not met. But this is also accidentally disabling IPsec in OVN which is not an expected behavior. This causes ovs-monitor-ipsec to refresh existing ipsec connections unnecessarily when IPsec pod comes up as it is not able to find remote_name from the tunnel. This may also trigger deleting IPsec connection entries from openshift.conf file if ovs-monitor-ipsec is not killed timely when ipsec daemonset is removed. So this commit enables ovn ipsec option as long as the API is set with Full mode. Signed-off-by: Periyasamy Palanisamy <pepalani@redhat.com>
pperiyasamy
added a commit
to pperiyasamy/cluster-network-operator
that referenced
this pull request
Jan 23, 2025
The following change on the machine pool `status.MachineCount == status.UpdatedMachineCount && hasSourceInMachineConfigStatus(status, machineConfigs)` is introduced with PR openshift#2349 which ensures IPsec machine config is always installed on all the nodes in the cluster, So this is deleting the IPsec daemonset as per the CNO state machine for IPsec when the condition is not met. But this is also accidentally disabling IPsec in OVN which is not an expected behavior. This causes ovs-monitor-ipsec to refresh existing ipsec connections unnecessarily when IPsec pod comes up as it is not able to find remote_name from the tunnel. This may also trigger deleting IPsec connection entries from openshift.conf file if ovs-monitor-ipsec is not killed timely when ipsec daemonset is removed. So this commit enables ovn ipsec option as long as the API is set with Full mode. Signed-off-by: Periyasamy Palanisamy <pepalani@redhat.com>
pperiyasamy
added a commit
to pperiyasamy/cluster-network-operator
that referenced
this pull request
Jan 28, 2025
The following change on the machine pool `status.MachineCount == status.UpdatedMachineCount && hasSourceInMachineConfigStatus(status, machineConfigs)` is introduced with PR openshift#2349 which ensures IPsec machine config is always installed on all the nodes in the cluster, So this is deleting the IPsec daemonset as per the CNO state machine for IPsec when the condition is not met. But this is also accidentally disabling IPsec in OVN which is not an expected behavior. This causes ovs-monitor-ipsec to refresh existing ipsec connections unnecessarily when IPsec pod comes up as it is not able to find remote_name from the tunnel. This may also trigger deleting IPsec connection entries from openshift.conf file if ovs-monitor-ipsec is not killed timely when ipsec daemonset is removed. So this commit enables ovn ipsec option as long as the API is set with Full mode. Signed-off-by: Periyasamy Palanisamy <pepalani@redhat.com>
pperiyasamy
added a commit
to pperiyasamy/cluster-network-operator
that referenced
this pull request
Feb 12, 2025
The following change on the machine pool `status.MachineCount == status.UpdatedMachineCount && hasSourceInMachineConfigStatus(status, machineConfigs)` is introduced with PR openshift#2349 which ensures IPsec machine config is always installed on all the nodes in the cluster, So this is deleting the IPsec daemonset as per the CNO state machine for IPsec when the condition is not met. But this is also accidentally disabling IPsec in OVN which is not an expected behavior. This causes ovs-monitor-ipsec to refresh existing ipsec connections unnecessarily when IPsec pod comes up as it is not able to find remote_name from the tunnel. This may also trigger deleting IPsec connection entries from openshift.conf file if ovs-monitor-ipsec is not killed timely when ipsec daemonset is removed. So this commit enables ovn ipsec option as long as the API is set with Full mode. Signed-off-by: Periyasamy Palanisamy <pepalani@redhat.com> (cherry picked from commit 4e57dcd)
pperiyasamy
added a commit
to pperiyasamy/cluster-network-operator
that referenced
this pull request
Feb 26, 2025
The following change on the machine pool `status.MachineCount == status.UpdatedMachineCount && hasSourceInMachineConfigStatus(status, machineConfigs)` is introduced with PR openshift#2349 which ensures IPsec machine config is always installed on all the nodes in the cluster, So this is deleting the IPsec daemonset as per the CNO state machine for IPsec when the condition is not met. But this is also accidentally disabling IPsec in OVN which is not an expected behavior. This causes ovs-monitor-ipsec to refresh existing ipsec connections unnecessarily when IPsec pod comes up as it is not able to find remote_name from the tunnel. This may also trigger deleting IPsec connection entries from openshift.conf file if ovs-monitor-ipsec is not killed timely when ipsec daemonset is removed. So this commit enables ovn ipsec option as long as the API is set with Full mode. Signed-off-by: Periyasamy Palanisamy <pepalani@redhat.com> (cherry picked from commit 4e57dcd) (cherry picked from commit 5323288)
pperiyasamy
added a commit
to pperiyasamy/cluster-network-operator
that referenced
this pull request
Mar 5, 2025
The following change on the machine pool `status.MachineCount == status.UpdatedMachineCount && hasSourceInMachineConfigStatus(status, machineConfigs)` is introduced with PR openshift#2349 which ensures IPsec machine config is always installed on all the nodes in the cluster, So this is deleting the IPsec daemonset as per the CNO state machine for IPsec when the condition is not met. But this is also accidentally disabling IPsec in OVN which is not an expected behavior. This causes ovs-monitor-ipsec to refresh existing ipsec connections unnecessarily when IPsec pod comes up as it is not able to find remote_name from the tunnel. This may also trigger deleting IPsec connection entries from openshift.conf file if ovs-monitor-ipsec is not killed timely when ipsec daemonset is removed. So this commit enables ovn ipsec option as long as the API is set with Full mode. Signed-off-by: Periyasamy Palanisamy <pepalani@redhat.com> (cherry picked from commit 4e57dcd) (cherry picked from commit 5323288)
pperiyasamy
added a commit
to pperiyasamy/cluster-network-operator
that referenced
this pull request
Mar 7, 2025
The following change on the machine pool `status.MachineCount == status.UpdatedMachineCount && hasSourceInMachineConfigStatus(status, machineConfigs)` is introduced with PR openshift#2349 which ensures IPsec machine config is always installed on all the nodes in the cluster, So this is deleting the IPsec daemonset as per the CNO state machine for IPsec when the condition is not met. But this is also accidentally disabling IPsec in OVN which is not an expected behavior. This causes ovs-monitor-ipsec to refresh existing ipsec connections unnecessarily when IPsec pod comes up as it is not able to find remote_name from the tunnel. This may also trigger deleting IPsec connection entries from openshift.conf file if ovs-monitor-ipsec is not killed timely when ipsec daemonset is removed. So this commit enables ovn ipsec option as long as the API is set with Full mode. Signed-off-by: Periyasamy Palanisamy <pepalani@redhat.com> (cherry picked from commit 4e57dcd) (cherry picked from commit 5323288)
pperiyasamy
added a commit
to pperiyasamy/cluster-network-operator
that referenced
this pull request
Mar 11, 2025
The following change on the machine pool `status.MachineCount == status.UpdatedMachineCount && hasSourceInMachineConfigStatus(status, machineConfigs)` is introduced with PR openshift#2349 which ensures IPsec machine config is always installed on all the nodes in the cluster, So this is deleting the IPsec daemonset as per the CNO state machine for IPsec when the condition is not met. But this is also accidentally disabling IPsec in OVN which is not an expected behavior. This causes ovs-monitor-ipsec to refresh existing ipsec connections unnecessarily when IPsec pod comes up as it is not able to find remote_name from the tunnel. This may also trigger deleting IPsec connection entries from openshift.conf file if ovs-monitor-ipsec is not killed timely when ipsec daemonset is removed. So this commit enables ovn ipsec option as long as the API is set with Full mode. Signed-off-by: Periyasamy Palanisamy <pepalani@redhat.com> (cherry picked from commit 4e57dcd)
pperiyasamy
added a commit
to pperiyasamy/cluster-network-operator
that referenced
this pull request
Mar 11, 2025
The following change on the machine pool `status.MachineCount == status.UpdatedMachineCount && hasSourceInMachineConfigStatus(status, machineConfigs)` is introduced with PR openshift#2349 which ensures IPsec machine config is always installed on all the nodes in the cluster, So this is deleting the IPsec daemonset as per the CNO state machine for IPsec when the condition is not met. But this is also accidentally disabling IPsec in OVN which is not an expected behavior. This causes ovs-monitor-ipsec to refresh existing ipsec connections unnecessarily when IPsec pod comes up as it is not able to find remote_name from the tunnel. This may also trigger deleting IPsec connection entries from openshift.conf file if ovs-monitor-ipsec is not killed timely when ipsec daemonset is removed. So this commit enables ovn ipsec option as long as the API is set with Full mode. Signed-off-by: Periyasamy Palanisamy <pepalani@redhat.com> (cherry picked from commit 4e57dcd) (cherry picked from commit 5323288) (cherry picked from commit 5dbefce)
pperiyasamy
added a commit
to pperiyasamy/cluster-network-operator
that referenced
this pull request
Mar 19, 2025
The following change on the machine pool `status.MachineCount == status.UpdatedMachineCount && hasSourceInMachineConfigStatus(status, machineConfigs)` is introduced with PR openshift#2349 which ensures IPsec machine config is always installed on all the nodes in the cluster, So this is deleting the IPsec daemonset as per the CNO state machine for IPsec when the condition is not met. But this is also accidentally disabling IPsec in OVN which is not an expected behavior. This causes ovs-monitor-ipsec to refresh existing ipsec connections unnecessarily when IPsec pod comes up as it is not able to find remote_name from the tunnel. This may also trigger deleting IPsec connection entries from openshift.conf file if ovs-monitor-ipsec is not killed timely when ipsec daemonset is removed. So this commit enables ovn ipsec option as long as the API is set with Full mode. Signed-off-by: Periyasamy Palanisamy <pepalani@redhat.com> (cherry picked from commit 4e57dcd)
pperiyasamy
added a commit
to pperiyasamy/cluster-network-operator
that referenced
this pull request
Mar 20, 2025
The following change on the machine pool `status.MachineCount == status.UpdatedMachineCount && hasSourceInMachineConfigStatus(status, machineConfigs)` is introduced with PR openshift#2349 which ensures IPsec machine config is always installed on all the nodes in the cluster, So this is deleting the IPsec daemonset as per the CNO state machine for IPsec when the condition is not met. But this is also accidentally disabling IPsec in OVN which is not an expected behavior. This causes ovs-monitor-ipsec to refresh existing ipsec connections unnecessarily when IPsec pod comes up as it is not able to find remote_name from the tunnel. This may also trigger deleting IPsec connection entries from openshift.conf file if ovs-monitor-ipsec is not killed timely when ipsec daemonset is removed. So this commit enables ovn ipsec option as long as the API is set with Full mode. Signed-off-by: Periyasamy Palanisamy <pepalani@redhat.com> (cherry picked from commit 4e57dcd) (cherry picked from commit 5323288)
pperiyasamy
added a commit
to pperiyasamy/cluster-network-operator
that referenced
this pull request
Mar 20, 2025
The following change on the machine pool `status.MachineCount == status.UpdatedMachineCount && hasSourceInMachineConfigStatus(status, machineConfigs)` is introduced with PR openshift#2349 which ensures IPsec machine config is always installed on all the nodes in the cluster, So this is deleting the IPsec daemonset as per the CNO state machine for IPsec when the condition is not met. But this is also accidentally disabling IPsec in OVN which is not an expected behavior. This causes ovs-monitor-ipsec to refresh existing ipsec connections unnecessarily when IPsec pod comes up as it is not able to find remote_name from the tunnel. This may also trigger deleting IPsec connection entries from openshift.conf file if ovs-monitor-ipsec is not killed timely when ipsec daemonset is removed. So this commit enables ovn ipsec option as long as the API is set with Full mode. Signed-off-by: Periyasamy Palanisamy <pepalani@redhat.com> (cherry picked from commit 4e57dcd) (cherry picked from commit 5323288) (cherry picked from commit 5dbefce)
pperiyasamy
added a commit
to pperiyasamy/cluster-network-operator
that referenced
this pull request
Mar 21, 2025
The following change on the machine pool `status.MachineCount == status.UpdatedMachineCount && hasSourceInMachineConfigStatus(status, machineConfigs)` is introduced with PR openshift#2349 which ensures IPsec machine config is always installed on all the nodes in the cluster, So this is deleting the IPsec daemonset as per the CNO state machine for IPsec when the condition is not met. But this is also accidentally disabling IPsec in OVN which is not an expected behavior. This causes ovs-monitor-ipsec to refresh existing ipsec connections unnecessarily when IPsec pod comes up as it is not able to find remote_name from the tunnel. This may also trigger deleting IPsec connection entries from openshift.conf file if ovs-monitor-ipsec is not killed timely when ipsec daemonset is removed. So this commit enables ovn ipsec option as long as the API is set with Full mode. Signed-off-by: Periyasamy Palanisamy <pepalani@redhat.com> (cherry picked from commit 4e57dcd) (cherry picked from commit 5323288) (cherry picked from commit 5dbefce) (cherry picked from commit 0ad1d0f)
pperiyasamy
added a commit
to pperiyasamy/cluster-network-operator
that referenced
this pull request
Mar 31, 2025
The following change on the machine pool `status.MachineCount == status.UpdatedMachineCount && hasSourceInMachineConfigStatus(status, machineConfigs)` is introduced with PR openshift#2349 which ensures IPsec machine config is always installed on all the nodes in the cluster, So this is deleting the IPsec daemonset as per the CNO state machine for IPsec when the condition is not met. But this is also accidentally disabling IPsec in OVN which is not an expected behavior. This causes ovs-monitor-ipsec to refresh existing ipsec connections unnecessarily when IPsec pod comes up as it is not able to find remote_name from the tunnel. This may also trigger deleting IPsec connection entries from openshift.conf file if ovs-monitor-ipsec is not killed timely when ipsec daemonset is removed. So this commit enables ovn ipsec option as long as the API is set with Full mode. Signed-off-by: Periyasamy Palanisamy <pepalani@redhat.com> (cherry picked from commit 4e57dcd) (cherry picked from commit 35ce486)
pperiyasamy
added a commit
to pperiyasamy/cluster-network-operator
that referenced
this pull request
Mar 31, 2025
The following change on the machine pool `status.MachineCount == status.UpdatedMachineCount && hasSourceInMachineConfigStatus(status, machineConfigs)` is introduced with PR openshift#2349 which ensures IPsec machine config is always installed on all the nodes in the cluster, So this is deleting the IPsec daemonset as per the CNO state machine for IPsec when the condition is not met. But this is also accidentally disabling IPsec in OVN which is not an expected behavior. This causes ovs-monitor-ipsec to refresh existing ipsec connections unnecessarily when IPsec pod comes up as it is not able to find remote_name from the tunnel. This may also trigger deleting IPsec connection entries from openshift.conf file if ovs-monitor-ipsec is not killed timely when ipsec daemonset is removed. So this commit enables ovn ipsec option as long as the API is set with Full mode. Signed-off-by: Periyasamy Palanisamy <pepalani@redhat.com> (cherry picked from commit 4e57dcd) (cherry picked from commit 35ce486) (cherry picked from commit 67bd2c3)
pperiyasamy
added a commit
to pperiyasamy/cluster-network-operator
that referenced
this pull request
Apr 2, 2025
The following change on the machine pool `status.MachineCount == status.UpdatedMachineCount && hasSourceInMachineConfigStatus(status, machineConfigs)` is introduced with PR openshift#2349 which ensures IPsec machine config is always installed on all the nodes in the cluster, So this is deleting the IPsec daemonset as per the CNO state machine for IPsec when the condition is not met. But this is also accidentally disabling IPsec in OVN which is not an expected behavior. This causes ovs-monitor-ipsec to refresh existing ipsec connections unnecessarily when IPsec pod comes up as it is not able to find remote_name from the tunnel. This may also trigger deleting IPsec connection entries from openshift.conf file if ovs-monitor-ipsec is not killed timely when ipsec daemonset is removed. So this commit enables ovn ipsec option as long as the API is set with Full mode. Signed-off-by: Periyasamy Palanisamy <pepalani@redhat.com> (cherry picked from commit 4e57dcd) (cherry picked from commit 35ce486) (cherry picked from commit 67bd2c3) (cherry picked from commit 2592e00)
This was referenced Apr 3, 2025
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.
While testing IPsec upgrade on 24 node cluster in CI, it's noticed there are more than two (master, worker) machine config pools present. For example infra and workload pools were present which belong worker role. Hence this PR queries all machine config pools based on master and worker roles and check ipsec plugin existence on every pool.