feat(test): use labels in meta e2e test #377
Conversation
Codecov Report
@@ Coverage Diff @@
## main #377 +/- ##
==========================================
- Coverage 71.61% 71.58% -0.04%
==========================================
Files 53 53
Lines 5641 5635 -6
==========================================
- Hits 4040 4034 -6
Misses 1529 1529
Partials 72 72
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
test/e2e/tests/risingwave/meta.sh
Outdated
| leader="$p" | ||
| # Iterate over the etcd election kv pairs. Delete leader lease if found, else abort the test | ||
| local del_lease=false | ||
| kubectl -n rwc-2-mytenant run del-leader-lease --image='anldisr/etcdctl' --restart='Never' --command=true -- etcdctl --endpoints=risingwave-etcd-mytenant:2379 get _ --prefix=true --write-out="json" |
There was a problem hiding this comment.
I believe we can utilize kubectl exec to execute the command without the need to fetch the image for this particular pod. For example:
$ k exec -it etcd-0 -- env ETCDCTL_API=3 etcdctl get a --prefix=true --write-out="json" --user=root:iGGUrEKBiX
{"header":{"cluster_id":16217987567572491554,"member_id":17870784734561721408,"revision":1,"raft_term":3}}In addition, the use of get _ could be overly aggressive and may result in timeouts. To avoid this issue, it would be better to implement lease list as an alternative solution.
There was a problem hiding this comment.
As always thank you for your guidance 😁
I totally agree with you, using exec instead of spewing a new pod is definitely better.
Regarding performance: I am not sure how we could use lease list in this case. That command returns
{
"cluster_id": 6604019301136558000,
"member_id": 1109445486625178000,
"revision": 13954,
"raft_term": 4,
"leases": [
{
"id": 4734838729492364000
},
{
"id": 4734838729492364000
},
{
"id": 4734838729492364000
}
]
}We receive the leases, but we do not know which keys are attached to these leases. I do not see how this benefits us here. Please let me know if I miss something.
I will use etcdctl get __meta_election_ --prefix=true from now on. Maybe that is a bit more performant, since we narrow down the search a bit more
There was a problem hiding this comment.
Never mind. I believe etcdctl get __meta_election_ --prefix=true already solves the problem. I didn't know more about the lease list than you.
test/e2e/tests/risingwave/meta.sh
Outdated
| del_lease=true | ||
| echo "found leader lease. Deleting it" | ||
| # delete leader lease | ||
| k8s::kubectl exec -it etcd-0 -- env ETCDCTL_API=3 etcdctl del "$(echo "$i" | jq -r .key | base64 --decode)" --user=root:iGGUrEKBiX |
There was a problem hiding this comment.
The iGGUrEKBiX is the password from my local... I presume it won't work in the tests. Could you try removing the --user and see if it works?
|
It seems the e2e test failed with an explicit reason: Could you help have a look @CAJan93? |
CAJan93
force-pushed
the
update_e2e_test
branch
from
5038873 to
46163b9
Compare
CAJan93
force-pushed
the
update_e2e_test
branch
from
46163b9 to
9de1311
Compare
…rator into update_e2e_test
|
I think this is blocked by risingwavelabs/risingwave#8238 |
…rator into update_e2e_test
…rator into update_e2e_test
|
Tried this with the |
…ingwave-operator into update_e2e_test
What's changed and what's your intention?
Follow-up from #352
Checklist
Refer to a related PR or issue link (optional)