-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Add tls for kine #9572
Add tls for kine #9572
Conversation
d427ad3
to
b3fd09c
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #9572 +/- ##
===========================================
- Coverage 53.04% 26.87% -26.17%
===========================================
Files 155 154 -1
Lines 13597 13614 +17
===========================================
- Hits 7212 3659 -3553
- Misses 5018 9215 +4197
+ Partials 1367 740 -627
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
a10a428
to
6fbfb32
Compare
6fbfb32
to
5aa7edb
Compare
5aa7edb
to
71e7fe5
Compare
bf950f1
to
f79ef25
Compare
9cd61fd
to
ddfe138
Compare
ddfe138
to
99fd6cc
Compare
618f7cd
to
9e1a5d7
Compare
/ # etcdctl endpoint status --cacert=/var/lib/rancher/k3s/server/tls/etcd/server-ca.crt --endpoints=unixs:///var/lib/rancher/k3s/server/kine.sock -w table
+-----------------------------------------------+----+---------+---------+-----------+------------+-----------+------------+--------------------+--------+
| ENDPOINT | ID | VERSION | DB SIZE | IS LEADER | IS LEARNER | RAFT TERM | RAFT INDEX | RAFT APPLIED INDEX | ERRORS |
+-----------------------------------------------+----+---------+---------+-----------+------------+-----------+------------+--------------------+--------+
| unixs:///var/lib/rancher/k3s/server/kine.sock | 0 | | 4.0 MB | true | false | 0 | 0 | 0 | |
+-----------------------------------------------+----+---------+---------+-----------+------------+-----------+------------+--------------------+--------+ I do see that kine is started twice:
However, it looks like both copies of kine are still running; we get duplicate/conflicting compaction messages at 5 minute intervals showing that they are both still active:
Are we not shutting down the initial non-tls instance of kine? When we start up the temporary etcd for bootstrapping, we do so with a bootstrap context that is cancelled once the data is extracted, so that etcd shuts down cleanly: Lines 490 to 503 in 1c8be1d
We need to do the same for kine so that the initial one doesn't remain running after bootstrapping is complete. |
9e1a5d7
to
f404c85
Compare
de50948
to
413c7e6
Compare
With the added context cancellation it looks like the extra kine is getting shut down properly, but the early shutdown does cause some additional errors to be emitted by kine:
Its possible that adding a short sleep before shutting down kine would prevent at least some of these errors from popping up? We do something similar when terminating etcd after a restore - although we probably don't need to wait 5 seconds as we do there. Lines 340 to 343 in fe2ca9e
It is also possible that you'd need to make changes on the kine side to not log errors if the err is due to context cancellation. |
62515c1
to
aa4bec7
Compare
aa4bec7
to
f3e6fe8
Compare
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.
lgtm!
f3e6fe8
to
12a5fe5
Compare
396958c
to
f2c948f
Compare
Signed-off-by: Vitor Savian <vitor.savian@suse.com> Bump kine Signed-off-by: Vitor Savian <vitor.savian@suse.com> Add integration tests for kine with tls Signed-off-by: Vitor Savian <vitor.savian@suse.com>
f2c948f
to
942f0fc
Compare
Proposed Changes
server/tls
folderTypes of Changes
Verification
--kine-tls
Testing
Linked Issues
User-Facing Change
Further Comments
This feature depends on a PR from kine
Also there is another PR that will help in the verification steps