09:01:12 <flwang1> #startmeeting magnum
09:01:13 <openstack> Meeting started Wed Mar 25 09:01:12 2020 UTC and is due to finish in 60 minutes.  The chair is flwang1. Information about MeetBot at http://wiki.debian.org/MeetBot.
09:01:14 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
09:01:16 <openstack> The meeting name has been set to 'magnum'
09:01:20 <flwang1> #topic roll call
09:01:23 <flwang1> o/
09:01:32 <strigazi> ο/
09:01:33 <flwang1> brtknr: ^
09:01:49 <brtknr> o/
09:02:24 <flwang1> i think just us
09:02:34 <flwang1> are you guys still safe?
09:02:50 <flwang1> NZ will lockdown in the next 4 weeks :(
09:02:55 <strigazi> all good here
09:03:06 <brtknr> yep, only left the house to go for a run yesterday but havent really left home for 2 weeks
09:03:07 <flwang1> be kind and stay safe
09:03:18 <brtknr> other than to go shopping
09:03:34 <brtknr> you too!
09:03:45 <flwang1> #topic update health status
09:03:56 <flwang1> thanks for the good review from brtknr
09:04:09 <flwang1> i think it's in a good shape now
09:04:44 <flwang1> and i have propose a PR in magnum auto healer https://github.com/kubernetes/cloud-provider-openstack/pull/985 if you want to give it a try
09:05:14 <brtknr> i think it would still be good to try and pursue updating the reason only and letting magnum conductor infer health_status
09:05:31 <flwang1> we (catalyst cloud) are keen to have this, because all our cluster are private and now we can't monitor their status
09:05:48 <brtknr> otherwise there will be multiple places with logic for determining health status
09:06:03 <flwang1> brtknr: we can, but how about do it in a separate, following patch?
09:06:07 <brtknr> also why make 2 calls to the API when you can do this with one?
09:07:07 <flwang1> i'm not feeling confident to change the internal health update logic in this patch
09:08:12 <flwang1> strigazi: thoughts?
09:08:46 <strigazi> I try to understand to which two calls you are talking about
09:09:10 <brtknr> 1 api call to update health_status and another api call to health_status_reason
09:09:30 <flwang1> brtknr: did we?
09:09:34 <strigazi> 1 call should be enough
09:09:40 <flwang1> brtknr: i forgot the details
09:10:22 <flwang1> I'm happy to improve it if it can be, my point is, i'd like to do it in a separate patch
09:10:30 <flwang1> instead of mixing in this patch
09:11:23 <strigazi> +1 to separate patch
09:11:42 <strigazi> (gerrit should allow many patches in a change)
09:11:51 <strigazi> (but it doesn't)
09:12:04 <flwang1> strigazi: please help review the current one, thanks
09:12:17 <flwang1> brtknr: are you ok with that?
09:12:31 <brtknr> also the internal poller is always setting the status to UNKNOWN
09:12:45 <brtknr> something needs to be done about that
09:13:26 <brtknr> otherwise it will be like a lottery
09:13:34 <brtknr> 50% of the time, the status will be UNKNOWN
09:13:47 <brtknr> which defeats the point of having an external updater
09:14:02 <flwang1> brtknr: can you explain?
09:14:12 <flwang1> it shouldn't be
09:14:37 <flwang1> i'm happy to fix it in the following patch
09:15:07 <brtknr> when I was testing it, the CLI would update the health_status, but the poller would reset it back to UNKNOWN
09:15:16 <flwang1> if both api and worker nodes are OK, then the health status should be healthy
09:15:35 <flwang1> brtknr: you're mixing the two things
09:16:07 <flwang1> are you saying there is a bug with the internal health status update logic?
09:16:41 <flwang1> i saw your patch for the corner case and I have already +2 on that, are you saying there is another potential bug?
09:16:44 <brtknr> there is no bug currently apart from the master_lb edge case
09:17:10 <flwang1> good
09:17:14 <brtknr> but if we want to be able to update externally, its a race condition between internal poller and the external update
09:17:28 <brtknr> the internal poller sets it back to UNKNOWN every polling interval?
09:17:32 <brtknr> make sense?
09:17:59 <flwang1> yep, but after we fixed the corner case by https://review.opendev.org/#/c/714589/, then we should be good
09:18:07 <flwang1> strigazi: can you help review https://review.opendev.org/#/c/714589/ ?
09:18:31 <flwang1> brtknr: are you ok we improve the health_status calculation in a separate patch?
09:19:13 <strigazi> If a magnum deployment is relying on an external thing, why not disalbe the conductor? I will have a look
09:20:01 <flwang1> strigazi: it's not hard depedency
09:20:11 <strigazi> it's not I know
09:20:24 <flwang1> we can introduce a config if you think that's better
09:20:42 <flwang1> a config to totally disable the internal polling for health status
09:20:53 <strigazi> I mean for someone who uses the external controller it makes sense
09:21:06 <flwang1> right
09:21:15 <strigazi> what brtknr proposes makes in this path
09:21:15 <brtknr> i am slightly unconfortable with it because if we have the health_status calculation logic in both CPO and magnum-conductor, we need to make 2 patches if we ever want to change this logic... my argument is that we should do this in one place... we already have this logic in magnum-conductor so makes sense to keep it therere and let the magnum-auto-healer simply provide the health_status_reason
09:21:19 <strigazi> what brtknr proposes makes in this patch
09:22:59 <brtknr> and let it calculate the health_status reason... i'm okay with it being a separate patch but i'd like to test them together
09:23:51 <flwang1> sure, i mean if the current patch is in good shape, we can get it in and which will make the following patch easy for testing and review
09:24:27 <flwang1> i just don't want to submit large patch because we don't have any function test in gate
09:25:12 <flwang1> as you know, we're fully relying on our manual testing to keep the magnum code quality
09:25:27 <flwang1> that's why i prefer to get smaller patch in
09:25:36 <flwang1> hopefully that makes sense for this case
09:26:06 <brtknr> ok makes sense
09:26:18 <brtknr> lets move to the next topic
09:26:24 <flwang1> thanks, let's move on
09:26:40 <flwang1> #topic https://review.opendev.org/#/c/714423/ - rootfs kubelet
09:27:06 <flwang1> brtknr: ^
09:27:21 <brtknr> ok so turns out mounting rootfs to kubelet fixes the cinder selinux issue
09:27:44 <brtknr> i tried mounting just the selinux specific things but that didnt help
09:28:05 <brtknr> selinux specific things: /sys/fs/selinx, /var/lib/selinux/, /etc/selinx
09:28:47 <strigazi> kubelet has access to the docker socket or another cri socket. The least privileged pattern made little sense here.
09:29:17 <brtknr> we mounted /rootfs to kubelet in atomic, strigazi suggested doing this ages ago but flwang and i were cautious, but we should take this
09:29:44 <flwang1> brtknr: after taking this, do we still have to disable selinux?
09:29:52 <brtknr> flwang1: nope
09:30:13 <brtknr> its upto you guys whether you want to take the selinux_mode patch
09:30:23 <brtknr> it might be useful for other things
09:30:40 <strigazi> the patch is useful
09:30:44 <flwang1> if that's the case, i prefer to mountfs and still enable seliux
09:31:25 <brtknr> ok :) lets take both then :P
09:31:51 <brtknr> selinux in fcos is always enabled by default
09:32:25 <flwang1> i'm ok with that
09:32:59 <flwang1> strigazi: ^
09:34:00 <strigazi> of I agree with it, optionally disabling a security feature (selinux) and giving extra access to an already super uber priliged process (kubelet)
09:34:25 <flwang1> cool
09:34:29 <flwang1> next topic?
09:34:48 <flwang1> #topic https://review.opendev.org/#/c/714574/ - cluster name for network
09:34:59 <flwang1> i'm happy to take this one
09:35:16 <flwang1> private as the network name is annoying sometimes
09:35:30 <brtknr> :)
09:35:35 <brtknr> glad you agree
09:36:32 <flwang1> strigazi: ^
09:36:40 <flwang1> anything else we need to discuss?
09:36:42 <strigazi> is it an issue when two clusters with the same name exist?
09:37:01 <flwang1> not a problem
09:37:11 <strigazi> we should do the same for subnets if not there
09:37:17 <brtknr> nope, it will be the same as when there are two networks called private
09:37:27 <brtknr> subnets get their name from heat stack
09:37:29 <flwang1> but sometimes it's not handy to find the correct network
09:37:45 <brtknr> e.g.  k8s-flannel-coreos-f2mpsj3k7y6i-network-2imn745rxgzv-private_subnet-27qmm3u76ubp
09:37:56 <brtknr> so its not a problem there
09:38:01 <strigazi> ok
09:38:14 <strigazi> makes sense
09:39:31 <flwang1> anything else we should discuss?
09:39:54 <brtknr> hmm i made a few patches yesterday
09:40:12 <brtknr> https://review.opendev.org/714719
09:40:20 <brtknr> changing repo for etcd
09:40:25 <brtknr> is that okay with you guys
09:40:49 <brtknr> i prefer quay.io/coreos as it uses the same release tag as etcd development repo
09:41:10 <brtknr> it annoys me that k8s.gcr.io drops the v from the release version
09:41:53 <flwang1> building etcd system container for atomic?
09:41:55 <brtknr> also on https://github.com/etcd-io/etcd/releases, they say they use quay.io/coreos/etcd as their secondanry container registry
09:42:10 <strigazi> where does the project publishes their builds? We should use that one (i don't know which one it is)
09:42:18 <brtknr> i am also okay to use gcr.io/etcd-development/etcd
09:42:45 <brtknr> according to https://github.com/etcd-io/etcd/releases, they publish to gcr.io/etcd-development/etcd and quay.io/coreos/etcd officially
09:43:12 <flwang1> i like quay.io since it's maintained by coreos
09:44:07 <brtknr> i am happy with either
09:44:13 <strigazi> I would choose the primary, but for us it doesn't matter, we mirror
09:44:34 <flwang1> agree
09:44:42 <flwang1> brtknr: done?
09:44:50 <flwang1> i have a question about metrics-server
09:44:55 <brtknr> okay shall i change it to primary or leave it as secondary?
09:45:14 <flwang1> when i run 'kubectl top node', i got :
09:45:14 <flwang1> Error from server (ServiceUnavailable): the server is currently unable to handle the request (get nodes.metrics.k8s.io)
09:45:37 <brtknr> is your metric server running?
09:46:01 <flwang1> yes
09:46:48 <brtknr> flwang1: do you have this patch: https://review.opendev.org/#/c/705984/
09:47:39 <flwang1> yes
09:47:39 <strigazi> what the metrics-server logs say?
09:48:30 <flwang1> http://paste.openstack.org/show/791116/
09:48:46 <flwang1> http://paste.openstack.org/show/791115/
09:49:03 <flwang1> i can't see much error from the metrics-server
09:49:04 <strigazi> which one?
09:49:13 <strigazi> 16 or 15
09:49:16 <flwang1> 791116
09:49:34 <brtknr> flwang1: is this master branch?
09:49:40 <flwang1> yes
09:49:56 <flwang1> i tested the caliao and coredns
09:50:06 <flwang1> maybe related to the calico issue
09:50:26 <flwang1> i will test it again with a master branch, no calico change
09:50:38 <flwang1> as for calico patch, strigazi, i do need your help
09:51:14 <flwang1> i think i have done everything and i can't see anything wrong, but the connection between nodes and pods don't work
09:51:40 <brtknr> flwang1: is this with calico plugin?
09:51:50 <brtknr> its not working for me either with calico
09:51:55 <flwang1> ok
09:51:59 <brtknr> probably to do with pod to pod communication issue
09:52:06 <brtknr> its working with flannel
09:52:12 <flwang1> then it should be the calico version upgrade issue
09:52:23 <strigazi> left this in gerrit too "With ip encapsualtion it works but the no-encapsulated mode is not working."
09:53:32 <brtknr> how do you enable ip encaptulation?
09:53:46 <brtknr> strigazi:
09:54:25 <flwang1> strigazi: just to be clear, you mean 'CALICO_IPV4POOL_IPIP' == 'Always' ?
09:54:26 <strigazi> https://review.opendev.org/#/c/705599/13/magnum/drivers/common/templates/kubernetes/fragments/calico-service.sh@454
09:54:42 <strigazi> Always
09:54:43 <strigazi> yes
09:55:07 <strigazi> Never should work though
09:55:14 <strigazi> as it used to work
09:55:32 <strigazi> when you have SDN on SDN this can happen :)
09:55:47 <strigazi> I mean being lost :)
09:56:12 <flwang1> strigazi: should we ask help for calico team?
09:56:18 <strigazi> yes
09:56:32 <flwang1> and i just found it hard to debug because the toobox doesn't work on fedora coreos
09:56:38 <strigazi> in devstack we run calico on openvswitch
09:56:42 <flwang1> so i can't use tcpdump to check the traffic
09:57:09 <flwang1> strigazi: did you try it on prod?
09:57:12 <flwang1> is it working?
09:57:14 <strigazi> flwang1: come on, privileged daemon with centos and instal whatever you want :)
09:57:35 <strigazi> s/daemon/daemonset/
09:57:55 <strigazi> or add a sidecar to calico node
09:57:56 <flwang1> strigazi: you mean just ran a centos daemon set?
09:58:19 <strigazi> or exec in calico node, it is RHEL
09:58:29 <strigazi> microdnf install
09:58:42 <flwang1> ok, will try
09:58:58 <flwang1> strigazi: did you try it on prod? is it working?
09:59:01 <strigazi> a sidecar is optimal
09:59:12 <strigazi> not yet, today, BUT
09:59:26 <strigazi> in prod we don't run on openvswitch
09:59:31 <strigazi> we use linux-bridge
09:59:39 <strigazi> so it may work
09:59:56 <strigazi> I will update gerrit
09:59:57 <flwang1> pls do, at least it can help us understand the issue
10:00:24 <flwang1> should I split the calico and coredns upgrade into 2 patches?
10:00:39 <brtknr> flwang1: probably good practice :)
10:00:42 <strigazi> as you want, it doesn't hurt
10:00:50 <flwang1> i combine them because they're very critical services
10:01:03 <flwang1> so i want to test them together for conformance test
10:01:04 <brtknr> they're not dependent on each other though right?
10:01:15 <flwang1> no depedency
10:01:15 <strigazi> they are not
10:01:40 <brtknr> have we ruled out if the regression is not caused by coredns?
10:01:49 <brtknr> have we ruled out if the regression is not caused by coredns upgrade?
10:01:55 <strigazi> if you update coredns can you make it run on master too?
10:02:00 <flwang1> i don't think it's related to coredns
10:02:05 <strigazi> it can't be
10:02:29 <strigazi> trust bu verify though
10:02:30 <flwang1> strigazi: make coredns only running on master node?
10:02:32 <strigazi> trust but verify though
10:02:46 <strigazi> flwang1: no, run in master as well
10:02:59 <brtknr> strigazi: why?
10:02:59 <flwang1> ah, sure, i can do that
10:03:13 <brtknr> why run on master as well?
10:03:23 <flwang1> brtknr: i even want to run it only on master ;)
10:03:36 <strigazi> because the user might have a stupid app that will run next to coredns and kill it
10:03:45 <flwang1> since it's critical service
10:04:00 <strigazi> then things on master don't have DNS
10:04:19 <flwang1> we don't want to lose it when the worker node down as well
10:04:47 <flwang1> let's end the meeting first
10:04:49 <brtknr> ok and I suppose we want it to run on workers too because we want the dns service to scale with the number of workers
10:05:00 <flwang1> #endmeeting