Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Bug 1837953: fixup server_id regex #694

Merged
merged 1 commit into from Jul 20, 2020
Merged

Bug 1837953: fixup server_id regex #694

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 8 additions & 4 deletions bindata/network/ovn-kubernetes/ovnkube-master.yaml
View file
Open in desktop
Expand Up @@ -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:]]+')
Copy link
Contributor

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-specific grep flags... Which may be a totally unreasonable expectation?

Copy link
Contributor Author

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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)
Copy link
Contributor Author

@rbbratta rbbratta Jul 20, 2020 •
edited

Choose a reason for hiding this comment

The 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}"
Expand Down Expand Up @@ -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}"
Expand Down