This commit implements a new mechanism to verify action-level
compatibility between controller and northd.
It provides a more flexible approach for versions upgrades than
the existing ovn-match-northd-version method.
The new feature allows for granular upgrades by focusing on action
compatibility rather than full version matching. This is particularly
valuable since southbound database schema changes are often backward
compatible, allowing controllers to continue functioning while only
requiring validation of action support.
Key changes:
- Added 'supported_actions' column to SB_Global table to track actions
supported by ovn-northd
- New 'validate-northd-actions' configuration flag for ovn-controller
- Runtime validation that compares controller and northd action support,
performed once during initial connection to the southbound database
and additionally triggered when the sb_global table is updated.
When enabled, ovn-controller validates that it supports all actions
generated by ovn-northd. If controller does not support some of the
actions available in ovn-northd, it will stop processing southbound
and local Open vSwitch database changes.
Signed-off-by: Alexandra Rukomoinikova <arukomoinikova@k2.cloud>
---
controller/ovn-controller.8.xml | 11 ++++
controller/ovn-controller.c | 108 +++++++++++++++++++++++++++++++-
include/ovn/actions.h | 9 +++
northd/en-global-config.c | 21 +++++++
ovn-sb.ovsschema | 7 ++-
ovn-sb.xml | 4 ++
tests/ovn.at | 58 +++++++++++++++++
7 files changed, 214 insertions(+), 4 deletions(-)
diff --git a/controller/ovn-controller.8.xml b/controller/ovn-controller.8.xml
index 32a1f7a6f..d31f1af50 100644
--- a/controller/ovn-controller.8.xml
+++ b/controller/ovn-controller.8.xml
@@ -407,6 +407,17 @@
being specified. NOTE: this feature is experimental and may be subject
to removal/change in the future.
</dd>
+
+ <dt><code>external_ids:validate-northd-actions</code></dt>
+ <dd>
+ The boolean flag indicates whether <code>ovn-controller</code>
+ needs to verify support for all actions that <code>ovn-northd</code>
+ supports.
+ If this flag is set to true and <code>ovn-controller</code> does not
+ support some of the actions available in ovn-northd,
+ it will stop processing southbound and local Open vSwitch database
+ changes.
+ </dd>
</dl>
<p>
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 6396fa898..f5935098f 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -180,6 +180,14 @@ struct pending_pkt {
/* Registered ofctrl seqno type for nb_cfg propagation. */
static size_t ofctrl_seq_type_nb_cfg;
+static struct validate_northd_actions {
+ bool controller_validate_actions;
+ bool validated;
+} validate_actions = {
+ .controller_validate_actions = false,
+ .validated = true
+};
+
static void
remove_newline(char *s)
{
@@ -210,6 +218,86 @@ static char *get_file_system_id(void)
free(filename);
return ret;
}
+
+static void
+init_validate_actions(struct ovsdb_idl *ovs_idl)
+{
+ const struct ovsrec_open_vswitch *cfg = ovsrec_open_vswitch_first(ovs_idl);
+ if (!cfg) {
+ return;
+ }
+ const struct ovsrec_open_vswitch_table *ovs_table =
+ ovsrec_open_vswitch_table_get(ovs_idl);
+ const char *chassis_id = get_ovs_chassis_id(ovs_table);
+
+ validate_actions.controller_validate_actions =
+ get_chassis_external_id_value_bool(&cfg->external_ids, chassis_id,
+ "validate-northd-actions", false);
+}
+
+static void
+log_actions_diff(const void *arg OVS_UNUSED,
+ const char *item, bool northd_supported)
+{
+ /* Controller-only actions are allowed, only
+ * generate informational log messages. */
+ if (northd_supported) {
+ validate_actions.validated = false;
+ }
+
+ VLOG_WARN("Validate actions: action %s supported by %s "
+ "but not supported by %s.", item,
+ northd_supported ? "northd" : "controller",
+ northd_supported ? "controller" : "northd");
+}
+
+static void
+validate_compatibility_with_northd(const struct sbrec_sb_global *sb,
+ bool sb_global_upgraded)
+{
+ static bool executed = false;
+ validate_actions.validated = true;
+
+ if (executed && !sb_global_upgraded) {
+ return;
+ }
+
+ if (!sb || !sb->n_supported_actions) {
+ VLOG_WARN("Validate actions: %s",
+ !sb ? "No SB Global record found." :
+ "No northd actions available in SB Global.");
+ validate_actions.validated = false;
+ return;
+ }
+
+ struct svec controller_action_entries = SVEC_EMPTY_INITIALIZER;
+ ovn_action_get_names(&controller_action_entries);
+
+ struct sorted_array controller_actions =
+ sorted_array_from_svec(&controller_action_entries);
+
+ struct sorted_array northd_actions =
+ sorted_array_create((const char **) sb->supported_actions,
+ sb->n_supported_actions,
+ false);
+
+ sorted_array_apply_diff(&northd_actions,
+ &controller_actions,
+ log_actions_diff, NULL);
+
+ VLOG_INFO("Validate actions: All northd actions are validated. ");
+
+ executed = sb_global_upgraded ? false : true;
+
+ if (!validate_actions.validated) {
+ engine_set_force_recompute();
+ }
+
+ sorted_array_destroy(&northd_actions);
+ sorted_array_destroy(&controller_actions);
+ svec_destroy(&controller_action_entries);
+}
+
/* Only set monitor conditions on tables that are available in the
* server schema.
*/
@@ -3629,6 +3717,10 @@ en_northd_options_run(struct engine_node *node, void
*data)
true)
: true;
+ if (validate_actions.controller_validate_actions) {
+ validate_compatibility_with_northd(sb_global, true);
+ }
+
return EN_UPDATED;
}
@@ -3686,6 +3778,10 @@ en_northd_options_sb_sb_global_handler(struct
engine_node *node, void *data)
result = EN_HANDLED_UPDATED;
}
+ if (validate_actions.controller_validate_actions) {
+ validate_compatibility_with_northd(sb_global, true);
+ }
+
return result;
}
@@ -7186,6 +7282,14 @@ main(int argc, char *argv[])
check_northd_version(ovs_idl_loop.idl, ovnsb_idl_loop.idl,
ovn_version);
+ init_validate_actions(ovs_idl_loop.idl);
+
+ if (validate_actions.controller_validate_actions) {
+ const struct sbrec_sb_global *sb =
+ sbrec_sb_global_first(ovnsb_idl_loop.idl);
+ validate_compatibility_with_northd(sb, false);
+ }
+
const struct ovsrec_bridge_table *bridge_table =
ovsrec_bridge_table_get(ovs_idl_loop.idl);
const struct ovsrec_open_vswitch_table *ovs_table =
@@ -7221,8 +7325,8 @@ main(int argc, char *argv[])
}
if (ovsdb_idl_has_ever_connected(ovnsb_idl_loop.idl) &&
- northd_version_match && cfg) {
-
+ northd_version_match && cfg &&
+ validate_actions.validated) {
/* Unconditionally remove all deleted lflows from the lflow
* cache.
*/
diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index 82d128f33..889055fb2 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -23,6 +23,7 @@
#include "expr.h"
#include "openvswitch/dynamic-string.h"
#include "openvswitch/hmap.h"
+#include "lib/svec.h"
#include "openvswitch/ofp-actions.h"
#include "openvswitch/uuid.h"
#include "util.h"
@@ -155,6 +156,14 @@ enum {
#undef OVNACT
};
+static inline void
+ovn_action_get_names(struct svec *ovnacts)
+{
+ #define OVNACT(ENUM, STRUCT) svec_add(ovnacts, #ENUM);
+ OVNACTS
+ #undef OVNACT
+}
+
/* Header for an action.
*
* Each action is a structure "struct ovnact_*" that begins with "struct
diff --git a/northd/en-global-config.c b/northd/en-global-config.c
index 76046c265..51a991ce5 100644
--- a/northd/en-global-config.c
+++ b/northd/en-global-config.c
@@ -26,6 +26,7 @@
#include "en-global-config.h"
#include "en-sampling-app.h"
#include "include/ovn/features.h"
+#include "include/ovn/actions.h"
#include "ipam.h"
#include "lib/ovn-nb-idl.h"
#include "lib/ovn-sb-idl.h"
@@ -51,6 +52,7 @@ static void update_sb_config_options_to_sbrec(struct
ed_type_global_config *,
const struct sbrec_sb_global *);
static bool is_vxlan_mode(const struct smap *nb_options,
const struct sbrec_chassis_table *);
+static void set_supported_actions_to_sbrec(const struct sbrec_sb_global *sb);
void *
en_global_config_init(struct engine_node *node OVS_UNUSED,
@@ -186,6 +188,8 @@ en_global_config_run(struct engine_node *node , void *data)
/* Set up SB_Global (depends on chassis features). */
update_sb_config_options_to_sbrec(config_data, sb);
+ set_supported_actions_to_sbrec(sb);
+
return EN_UPDATED;
}
@@ -720,3 +724,20 @@ is_vxlan_mode(const struct smap *nb_options,
}
return false;
}
+
+static void
+set_supported_actions_to_sbrec(const struct sbrec_sb_global *sb)
+{
+ struct svec ovnacts = SVEC_EMPTY_INITIALIZER;
+
+ ovn_action_get_names(&ovnacts);
+
+ struct sorted_array ovnacts_sorted =
+ sorted_array_from_svec(&ovnacts);
+
+ sbrec_sb_global_set_supported_actions(sb, ovnacts_sorted.arr,
+ ovnacts_sorted.n);
+
+ sorted_array_destroy(&ovnacts_sorted);
+ svec_destroy(&ovnacts);
+}
diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema
index f64cb99dd..9120e0d61 100644
--- a/ovn-sb.ovsschema
+++ b/ovn-sb.ovsschema
@@ -1,7 +1,7 @@
{
"name": "OVN_Southbound",
- "version": "21.4.0",
- "cksum": "812831561 35225",
+ "version": "21.4.1",
+ "cksum": "1176053856 35400",
"tables": {
"SB_Global": {
"columns": {
@@ -18,6 +18,9 @@
"type": {"key": {"type": "uuid",
"refTable": "SSL"},
"min": 0, "max": 1}},
+ "supported_actions" : {"type": {"key": "string",
+ "min": 0,
+ "max": "unlimited"}},
"options": {
"type": {"key": "string", "value": "string",
"min": 0, "max": "unlimited"}},
diff --git a/ovn-sb.xml b/ovn-sb.xml
index 4b563c5f1..fc2e5a3f6 100644
--- a/ovn-sb.xml
+++ b/ovn-sb.xml
@@ -165,6 +165,10 @@
<column name="options">
</column>
+
+ <column name="supported_actions">
+ Logical flow actions supported by northd.
+ </column>
</group>
<group title="Common options">
diff --git a/tests/ovn.at b/tests/ovn.at
index 292ca0dae..39a00ebb7 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -43872,3 +43872,61 @@ wait_for_ports_up sw0-vir
OVN_CLEANUP([hv1])
AT_CLEANUP
])
+
+OVN_FOR_EACH_NORTHD_NO_HV([
+AT_SETUP([Controller-northd action compatibility validation])
+ovn_start
+
+net_add n1
+
+sim_add hv1
+as hv1
+ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.1
+ovs-vsctl \
+ -- add-port br-int vif1 \
+ ofport-request=1
+
+ovs-vsctl \
+ -- add-port br-int vif2 \
+ -- set Interface vif2 external_ids:iface-id=lp2 \
+ ofport-request=2
+
+check ovn-nbctl ls-add ls1
+check ovn-nbctl lsp-add ls1 lp1
+check ovn-nbctl lsp-add ls1 lp2
+check ovn-nbctl lsp-add ls1 lp3
+
+# Wait for port to be bound.
+wait_row_count Chassis 1 name=hv1
+ch=$(fetch_column Chassis _uuid name=hv1)
+wait_row_count Port_Binding 1 logical_port=lp2 chassis=$ch
+
+AT_CHECK([ovn-sbctl get SB_Global . options:log-pipeline-ingress-len],
+[0], ["32"
+])
+
+AT_CHECK([ovn-sbctl get SB_Global . options:log-pipeline-egress-len],
+[0], ["14"
+])
+
+
+# Since from the tests we can't check the situation when the controller
doesn't support all
+# northd actions - we will check the opposite situation.
+check ovs-vsctl set Open_Vswitch . external_ids:validate-northd-actions=true
+
+check ovn-sbctl remove SB_G . supported_actions MOVE
+
+AT_CHECK([grep -o "action MOVE supported by controller but not supported by
northd." hv1/ovn-controller.log], [0], [dnl
+action MOVE supported by controller but not supported by northd.
+])
+
+as hv1 ovs-vsctl \
+ -- add-port br-int vif1 \
+ -- set Interface vif1 external_ids:iface-id=lp1 \
+ ofport-request=1
+
+wait_row_count Port_Binding 1 logical_port=lp1 chassis=$ch
+
+AT_CLEANUP
+])
--
2.48.1
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
