add vtbackup extraflags support #720
Conversation
jdoupe
force-pushed
the
add-vtbackup-extraflags
branch
2 times, most recently
from
2a84146 to
2008e1e
Compare
frouioui
left a comment
frouioui
left a comment
There was a problem hiding this comment.
This is a good start, thank you. Could you add a test for this please?
jdoupe
force-pushed
the
add-vtbackup-extraflags
branch
from
2008e1e to
89242c4
Compare
I added a test. Not a lot of examples to work with, and also wasn't really able to execute on my laptop, so hopefully it works well. I may have to work on an alternate development / test environment if it has issues. Resolved comments. Made things a bit cleaner. Thank you. |
| if !strings.Contains(joined, "--foo=bar") { | ||
| f.Fatalf("vtbackup args missing --foo=bar: %v", args) | ||
| } | ||
| if !strings.Contains(joined, "--baz=qux") { | ||
| f.Fatalf("vtbackup args missing --baz=qux: %v", args) | ||
| } |
There was a problem hiding this comment.
| if !strings.Contains(joined, "--foo=bar") { | |
| f.Fatalf("vtbackup args missing --foo=bar: %v", args) | |
| } | |
| if !strings.Contains(joined, "--baz=qux") { | |
| f.Fatalf("vtbackup args missing --baz=qux: %v", args) | |
| } | |
| require.Contains(t, joined, "--foo=bar", "vtbackup args missing --foo=bar: %v", args) | |
| require.Contains(t, joined, "--baz=qux", "vtbackup args missing --baz=qux: %v", args) |
There was a problem hiding this comment.
I blindly updated this, however, I'll note that "require" is currently only also used in test/integration/framework/fixture.go. The strings form I used originally is used elsewhere.
Am I to assume this is just the direction you want to move, and thus the suggested change?
jdoupe
force-pushed
the
add-vtbackup-extraflags
branch
from
a0d1816 to
da9317f
Compare
Rebased instead... just to be sure. |
Signed-off-by: Jeremy Doupe <[email protected]>
jdoupe
force-pushed
the
add-vtbackup-extraflags
branch
from
da9317f to
f9b8dc4
Compare
|
@jdoupe it looks like the test is failing. Reach out to me in the Vitess Slack and I'll be happy to walk through how you can run the tests locally to see what's failing and why. |
Signed-off-by: Jeremy Doupe <[email protected]>
jdoupe
force-pushed
the
add-vtbackup-extraflags
branch
from
2d024ff to
3b2dd63
Compare
mattlord
left a comment
mattlord
left a comment
There was a problem hiding this comment.
LGTM! Thanks, @jdoupe !
I'm curious if you've also done any local testing with Minikube or anything as well? It's worth doing IMO: https://vitess.io/docs/24.0/get-started/operator/
But not required.
I have! And I'm also running this in production on two small clusters. (patched version of v2.15.1) |
3 participants
Addresses #719