Skip to content

[WIP][DNM]Ic alert refactor#1878

Closed
bpickard22 wants to merge 8 commits intoopenshift:masterfrom
bpickard22:ic-alert-refactor
Closed

[WIP][DNM]Ic alert refactor#1878
bpickard22 wants to merge 8 commits intoopenshift:masterfrom
bpickard22:ic-alert-refactor

Conversation

@bpickard22
Copy link
Contributor

PR to allow for review only, this commit will be given to @ricky-rav as part of his work in CNO for ovn-ic

@ricky-rav i need some eyes on the labels for the service monitor to make sure I am using the right syntax

Also need someone from the monitoring team to take a look at the alert expressions to make sure I am going about that correctly

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 7, 2023
@openshift-ci openshift-ci bot requested review from abhat and tssurya July 7, 2023 23:08
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 7, 2023

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: bpickard22
Once this PR has been reviewed and has the lgtm label, please assign trozet for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@bpickard22 bpickard22 force-pushed the ic-alert-refactor branch from bde2828 to 0989e92 Compare July 8, 2023 00:50
@tssurya
Copy link
Contributor

tssurya commented Jul 10, 2023

@bpickard22 : please rebase this on top of #1838 (meaning include Ric's commits and then yours as well - otherwise we won't be able to see the CI on the actual thing before you hand it to Ric and he will have to fix stuff) : I'd like to see a green CI on Interconnect, as far as alerts are concerned.

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 10, 2023
ricky-rav and others added 8 commits July 10, 2023 10:56
- Determine OVN interconnect zone mode by inspecting an (optional) configMap; apply the desired zone mode.
- upgrade from non-IC to IC OVN-K by going through an intermediate step with 1-zone
- Switch from IC single zone to IC multizone (as in upgrades) and back (not fully supported yet, for internal use only)

Avoid clashes between single-zone ovnkube-master (using ports 9102, 9641, 9642, 29102) and multizone ovnkube-node (initially using ports 9103, 9105, 9102, 29102, 29103) during upgrade from 4.13 and avoid using ports reserved for the storage components, as described in https://github.com/openshift/enhancements/blob/master/dev-guide/host-port-registry.md  This caused the storage operator to never be available after installation of or upgrade to 4.14.

In multizone ovnkube-node let's now have:
- 9103, 9105, 29103 (which don't collide with single-zone ovnkube-master)
- 9112, 9112 9113, 29113 so as to not collide with single-zone ovnkube-master

Signed-off-by: Riccardo Ravaioli <rravaiol@redhat.com>
Signed-off-by: Riccardo Ravaioli <rravaiol@redhat.com>
Signed-off-by: Riccardo Ravaioli <rravaiol@redhat.com>
In the very last step of the 2-phase upgrade to OVN interconnect, we remove the IC configmap.
At this point, SetFromPods from pod_status.go won't be called any more, because all changes to the daemonsets have been processed. Patch the ovnk master daemonset with a dummy annotation to trigger status recalculation.

TODO: find a better way to run SetFromPods instead of updating ovnk master annotations

Signed-off-by: Riccardo Ravaioli <rravaiol@redhat.com>
Signed-off-by: Riccardo Ravaioli <rravaiol@redhat.com>
In ovn-ic there will be no raft as each node will have its own instance
of the databases, so we can remove the raft related alerts

Signed-off-by: Ben Pickard <bpickard@redhat.com>
Refactor alerts to run based off new metric names in ovn-ic

Signed-off-by: Ben Pickard <bpickard@redhat.com>
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 10, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 10, 2023

@bpickard22: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-aws-ovn-serial 4289b01 link false /test e2e-aws-ovn-serial
ci/prow/e2e-aws-sdn-network-reverse-migration 4289b01 link true /test e2e-aws-sdn-network-reverse-migration
ci/prow/e2e-network-mtu-migration-ovn-ipv4 4289b01 link false /test e2e-network-mtu-migration-ovn-ipv4
ci/prow/e2e-azure-ovn 4289b01 link false /test e2e-azure-ovn
ci/prow/e2e-ovn-ipsec-step-registry 4289b01 link false /test e2e-ovn-ipsec-step-registry
ci/prow/e2e-vsphere-ovn 4289b01 link false /test e2e-vsphere-ovn
ci/prow/e2e-metal-ipi-ovn-ipv6 4289b01 link true /test e2e-metal-ipi-ovn-ipv6
ci/prow/e2e-aws-sdn-upgrade 4289b01 link false /test e2e-aws-sdn-upgrade
ci/prow/unit 4289b01 link true /test unit
ci/prow/e2e-vsphere-ovn-dualstack 4289b01 link false /test e2e-vsphere-ovn-dualstack
ci/prow/e2e-gcp-ovn-upgrade 4289b01 link false /test e2e-gcp-ovn-upgrade
ci/prow/e2e-ovn-step-registry 4289b01 link false /test e2e-ovn-step-registry
ci/prow/e2e-openstack-ovn 4289b01 link false /test e2e-openstack-ovn
ci/prow/e2e-gcp-ovn 4289b01 link true /test e2e-gcp-ovn
ci/prow/e2e-metal-ipi-ovn-ipv6-ipsec 4289b01 link false /test e2e-metal-ipi-ovn-ipv6-ipsec
ci/prow/e2e-network-mtu-migration-ovn-ipv6 4289b01 link false /test e2e-network-mtu-migration-ovn-ipv6
ci/prow/e2e-ovn-hybrid-step-registry 4289b01 link false /test e2e-ovn-hybrid-step-registry
ci/prow/e2e-aws-ovn-network-migration 4289b01 link true /test e2e-aws-ovn-network-migration

Full PR test history. Your PR dashboard.

Details

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.

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 23, 2023
@openshift-merge-robot
Copy link
Contributor

PR needs rebase.

Details

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.

@bpickard22 bpickard22 closed this Jul 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants