New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bug 1837953: fixup server_id regex #694
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -123,8 +123,10 @@ spec: | |
return | ||
fi | ||
echo "Checking ${db} health" | ||
serverid=$(ovsdb-tool show-log "${db}" | sed -ne '/server_id:/s/server_id: *\([[:xdigit:]]\+\)/\1/p') | ||
match=$(ovsdb-tool show-log "${db}" | grep "prev_servers: *${serverid}(" || true) | ||
serverid=$(ovsdb-tool show-log "${db}" | grep -m 1 -oP '(?<=server_id: )[[:xdigit:]]+') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. https://github.com/openvswitch/ovs/blob/master/ovsdb/ovsdb-tool.c#L881 printf(" server_id: "SID_FMT"\n", SID_ARGS(&h->sid)); https://github.com/openvswitch/ovs/blob/master/ovsdb/raft-private.h#L36 #define SID_FMT "%04x" |
||
# prev_servers is a list. We are searching for " $server_id(" in the list | ||
# prev_servers: 33b4("ssl:10.19.138.37:9643"), 720d("ssl:10.19.138.33:9643"), 7e4f("ssl:10.19.138.32:9643") | ||
match=$(ovsdb-tool show-log "${db}" | grep "prev_servers:.* ${serverid}(" || true) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. https://github.com/openvswitch/ovs/blob/master/ovsdb/ovsdb-tool.c#L897 print_servers("prev_servers", h->snap.servers); https://github.com/openvswitch/ovs/blob/master/ovsdb/ovsdb-tool.c#L828 printf(" %s: ", name);
const struct shash_node **nodes = shash_sort(json_object(servers));
size_t n = shash_count(json_object(servers));
for (size_t i = 0; i < n; i++) {
if (i > 0) {
printf(", ");
}
const struct shash_node *node = nodes[i];
printf("%.4s(", node->name);
const struct json *address = node->data;
char *s = json_to_string(address, JSSF_SORT);
fputs(s, stdout);
free(s);
putchar(')');
} |
||
if [[ -z ${match} ]]; then | ||
echo "Current server_id $serverid not found in ${db}, cleaning up" | ||
rm -- "${db}" | ||
|
@@ -311,8 +313,10 @@ spec: | |
return | ||
fi | ||
echo "Checking ${db} health" | ||
serverid=$(ovsdb-tool show-log "${db}" | sed -ne '/server_id:/s/server_id: *\([[:xdigit:]]\+\)/\1/p') | ||
match=$(ovsdb-tool show-log "${db}" | grep "prev_servers: *${serverid}(" || true) | ||
serverid=$(ovsdb-tool show-log "${db}" | grep -m 1 -oP '(?<=server_id: )[[:xdigit:]]+') | ||
# prev_servers is a list. We are searching for " $server_id(" in the list | ||
# prev_servers: 33b4("ssl:10.19.138.37:9643"), 720d("ssl:10.19.138.33:9643"), 7e4f("ssl:10.19.138.32:9643") | ||
match=$(ovsdb-tool show-log "${db}" | grep "prev_servers:.* ${serverid}(" || true) | ||
if [[ -z ${match} ]]; then | ||
echo "Current server_id $serverid not found in ${db}, cleaning up" | ||
rm -- "${db}" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤷♂️ I like the original version better... I feel like it's reasonable to expect future people reading this to understand
sed
syntax but not reasonable to expect them to know GNU-specificgrep
flags... Which may be a totally unreasonable expectation?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
grep -o
seems cleaner instead of trying to remove all the text surrounding the match. It feels kinda sad that GNU maintainers add all these nice features (-o
and PCRE, for example, ~20 years ago) and no one uses them.