Manageiq: Worker deployments exist after worker records are removed
When deploying manageiq in openshift or kubernetes it's possible that worker deployments are left in the namespace when the worker rows have been deleted.
This means that the server no longer knows these workers are running so it also doesn't know to bring them down.
Here's an example:
$ oc get deploy | grep 1-
1-amazon-agent-coordinator-0 1/1 1 1 6h24m
1-amazon-cloud-event-catcher-135 1/1 1 1 73m
1-amazon-cloud-event-catcher-144 1/1 1 1 73m
1-amazon-cloud-metrics-collector 2/2 2 2 41h
1-amazon-cloud-refresh-10-8-9 0/1 1 0 2d19h
1-amazon-cloud-refresh-135-136-137 1/1 1 1 6h25m
1-amazon-cloud-refresh-144-145-146 1/1 1 1 74m
1-amazon-cloud-refresh-18-19-20 0/1 1 0 2d9h
1-amazon-cloud-refresh-84-85-86 0/1 1 0 25h
1-amazon-cloud-refresh-89-90-91 0/1 1 0 25h
1-azure-cloud-event-catcher-59 0/1 1 0 28h
1-azure-cloud-metrics-collector 2/2 2 2 32h
1-azure-cloud-refresh-67-68 1/1 1 1 27h
1-azure-cloud-refresh-69-70 1/1 1 1 27h
1-ems-metrics-processor 2/2 2 2 41h
1-event-handler 1/1 1 1 3d
1-generic 2/2 2 2 3d
1-google-cloud-event-catcher-16 0/1 1 0 2d9h
1-google-cloud-event-catcher-35 1/1 1 1 2d4h
1-google-cloud-event-catcher-39 1/1 1 1 46h
1-google-cloud-event-catcher-98 1/1 1 1 23h
1-google-cloud-metrics-collector 2/2 2 2 41h
1-google-cloud-refresh-16 0/1 1 0 2d9h
1-google-cloud-refresh-35 1/1 1 1 2d4h
1-google-cloud-refresh-39 1/1 1 1 46h
1-google-cloud-refresh-98 1/1 1 1 23h
1-google-network-refresh-17 1/1 1 1 2d9h
1-google-network-refresh-36 1/1 1 1 2d4h
1-google-network-refresh-40 1/1 1 1 46h
1-google-network-refresh-99 1/1 1 1 23h
1-openshift-container-event-catcher-150 1/1 1 1 37m
1-openshift-container-metrics-collector 2/2 2 2 37m
1-openshift-container-refresh-150 1/1 1 1 37m
1-priority 2/2 2 2 3d
1-remote-console 1/1 1 1 3d
1-reporting 2/2 2 2 3d
1-schedule 1/1 1 1 3d
1-ui 1/1 1 1 3d
1-web-service 1/1 1 1 3d
vmdb_production=# select count(id), type, array_agg(queue_name) from miq_workers group by type order by type;
count | type | array_agg
-------+--------------------------------------------------------------------------+---------------------------------------------------------------------------
------------
1 | ManageIQ::Providers::Amazon::AgentCoordinatorWorker | {ems_0}
5 | ManageIQ::Providers::Amazon::CloudManager::EventCatcher | {ems_2,ems_5,ems_135,ems_144,ems_147}
2 | ManageIQ::Providers::Amazon::CloudManager::MetricsCollectorWorker | {amazon,amazon}
2 | ManageIQ::Providers::Amazon::CloudManager::RefreshWorker | {"[\"ems_144\", \"ems_145\", \"ems_146\"]","[\"ems_135\", \"ems_136\", \"e
ms_137\"]"}
2 | ManageIQ::Providers::Azure::CloudManager::EventCatcher | {ems_67,ems_69}
2 | ManageIQ::Providers::Azure::CloudManager::MetricsCollectorWorker | {azure,azure}
2 | ManageIQ::Providers::Azure::CloudManager::RefreshWorker | {"[\"ems_67\", \"ems_68\"]","[\"ems_69\", \"ems_70\"]"}
3 | ManageIQ::Providers::Google::CloudManager::EventCatcher | {ems_35,ems_39,ems_98}
2 | ManageIQ::Providers::Google::CloudManager::MetricsCollectorWorker | {google,google}
3 | ManageIQ::Providers::Google::CloudManager::RefreshWorker | {ems_39,ems_98,ems_35}
3 | ManageIQ::Providers::Google::NetworkManager::RefreshWorker | {ems_36,ems_99,ems_40}
1 | ManageIQ::Providers::Openshift::ContainerManager::EventCatcher | {ems_150}
2 | ManageIQ::Providers::Openshift::ContainerManager::MetricsCollectorWorker | {openshift,openshift}
1 | ManageIQ::Providers::Openshift::ContainerManager::RefreshWorker | {ems_150}
2 | MiqEmsMetricsProcessorWorker | {ems_metrics_processor,ems_metrics_processor}
1 | MiqEventHandler | {ems}
2 | MiqGenericWorker | {generic,generic}
2 | MiqPriorityWorker | {generic,generic}
1 | MiqRemoteConsoleWorker | {NULL}
2 | MiqReportingWorker | {reporting,reporting}
1 | MiqScheduleWorker | {NULL}
2 | MiqSmartProxyWorker | {smartproxy,smartproxy}
1 | MiqUiWorker | {NULL}
1 | MiqWebServiceWorker | {NULL}
(24 rows)
In this case the deployment 1-amazon-cloud-refresh-10-8-9 should not exist as there is no amazon refresh worker with queues ems_8, ems_9, and ems_10.
For this we need to figure out how this deployment didn't get deleted when the worker was removed then either solve that problem or create some generic cleanup function that will make the deployments match what is in the workers table.
carbonin
All 10 comments
Completely unrelated but @agrare why do the RefreshWorker array_agg entries look so different from the rest? Is it doing some sort of stringified-array?
Fryguy
on 8 May 2020
@Fryguy that is the "child-manager refresher consolidation" so that the CloudManager/NetworkManager/StorageManager are all refreshed by one worker.
agrare
on 8 May 2020
It's possible this issue was caused by this which really should never happen:
[----] E, [2020-06-10T17:31:25.682621 #7:2af2278cb95c] ERROR -- : [Kubeclient::HttpError]: deployments.apps "1-amazon-cloud-event-catcher-17" is forbidden: User "system:serviceaccount:manageiq:default" cannot patch resource "deployments" in API group "apps" in the namespace "manageiq" Method:[block (2 levels) in <class:LogProxy>]
[----] E, [2020-06-10T17:31:25.683400 #7:2af2278cb95c] ERROR -- : gems/kubeclient-4.1.2/lib/kubeclient/common.rb:125:in `rescue in handle_exception'
gems/kubeclient-4.1.2/lib/kubeclient/common.rb:115:in `handle_exception'
gems/kubeclient-4.1.2/lib/kubeclient/common.rb:381:in `patch_entity'
gems/kubeclient-4.1.2/lib/kubeclient/common.rb:238:in `block (2 levels) in define_entity_methods'
gems/kubeclient-4.1.2/lib/kubeclient/common.rb:96:in `method_missing'
/var/www/miq/vmdb/lib/container_orchestrator.rb:15:in `scale'
/var/www/miq/vmdb/lib/container_orchestrator.rb:43:in `delete_deployment'
/var/www/miq/vmdb/app/models/miq_worker/deployment_per_worker.rb:13:in `delete_container_objects'
/var/www/miq/vmdb/app/models/miq_worker.rb:399:in `kill_process'
/var/www/miq/vmdb/app/models/miq_worker.rb:393:in `kill'
gems/activerecord-5.2.4.3/lib/active_record/relation/delegation.rb:71:in `each'
gems/activerecord-5.2.4.3/lib/active_record/relation/delegation.rb:71:in `each'
/var/www/miq/vmdb/app/models/ext_management_system.rb:585:in `destroy'
/var/www/miq/vmdb/app/models/miq_queue.rb:484:in `block in dispatch_method'
These logs show that the event catcher generic worker is calling ExtManagementSystem#destroy which directly attempts to run delete_container_objects (this will always fail). I'm still not sure how this would lead to the worker record actually getting removed, but it seems plausible.
To avoid something like this from happening we really should queue the calls to delete_container_objects for the server/orchestrator. The trick is ensuring that the worker instance isn't destroyed before the server picks the message up.
carbonin
on 10 Jun 2020
do you see log.info("SQL Record for #{w.format_full_log_msg}, Status: [#{w.status}] is being deleted") in the logs?
The other way the pod can remain but the worker row removed is if this exception is raised and they restart the "orchestrator" as I believe that still kills the existing worker rows at startup, right?
jrafanie
on 11 Jun 2020
@jrafanie I wonder if the worker deletes it's row if it crashes?
ref: https://github.com/ManageIQ/manageiq/blob/master/lib/workers/bin/run_single_worker.rb#L115-L117
carbonin
on 11 Jun 2020
haha, a 3rd option.
clean_worker_recordslogs the message I posted above...check_not_respondingdoesn't log but it also shouldn't be in play with the big oldreturn if MiqEnvironment::Command.is_podified?guard clause at the frontrun_single_worker.rbas you found also logs the delete when the worker runner exits safely or unsafely.
I think we should get a message in the log if either of the possible options happened.
jrafanie
on 11 Jun 2020
So I reproduced the generic worker failure by just adding and removing a provider, but the worker deployments did get removed so even though it's definitely a problem, it might be unrelated.
carbonin
on 11 Jun 2020
So I think we're going to go with the "create some generic cleanup function that will make the deployments match what is in the workers table" option from the issue description.
How
From a high level we need to be able to correlate the number of workers in kube with the number that we expect (from the database). We can get this info from kube by finding the number of replicas for worker types that support multiple replicas and comparing directly to the deployments for the "deployment per worker" case.
We should label our worker deployments consistently so that they're easy to search for. I think the easiest thing to do would be to label all our deployments with some common info then sort through those deployments in memory. That's similar to what we did in topological-inventory.
Here we get the deployment configs with a generic with a label selector using kubeclient, here we process those returned values for more specific information and here is where we define the labels on creation.
I think this is the way to go because the number of deployments will be relatively small so it doesn't make sense to do a round trip to the OpenShift api for each worker type or something similar.
As for what the labels should be we should have something simple like manageiq/worker=true maybe? That would allow us to pull back all the worker deployments in one call. Then we would need to be able to identify them by worker class name and possibly by ems id (for the ems case). We could do that with the label value, an annotation, or reversing the deployment name. I vote for an annotation here, but any of those would work.
Once we have all the information from kube we probably just want to search for deployments that shouldn't be present rather than also trying to reconcile the number of replicas. I think the logic at this point would be for each deployment to query if we have any workers matching workers in the table.
What
The logic at that point is up for debate. For provider workers, we should be able to determine if we have a matching ems. If we do, the worker stays (it likely is crashing for some valid reason, or just hasn't created its worker record yet). If we don't have a matching provider record it goes (this would mean we're in some strange situation where the worker deployment would never be removed).
I'm not really sure what to do for non-provider workers. I don't think we've ever encountered this issue for that case and I'm not sure that we would be able to tell the difference between something that was just coming up and something that shouldn't be present (there wouldn't be a worker record in either case). We could evaluate the roles and settings and all of that over again, but that seems like overkill. Maybe we start with just doing this for provider workers then expand to others if we need.
When
This could be a new server method called from the monitor loop, but the more I think about the logic, the more it feels like it makes sense right next to the existing worker reconciliation. We're going to have the information by worker type so maybe we should bake it into sync_workers. I think this decision can be made later depending on how the implementation shakes out.
carbonin
on 23 Jul 2020
I think this would help the systemd side as well to catch essentially the same issue, service/unit files for workers that shouldn't be running, :+1: sounds good to me
agrare
on 24 Jul 2020
Thinking about this more, it feels like this would be a good first step to removing the workers table (or at least its role in reconciliation). The problem we have here is that we're treating the table as "reality" when really we can inspect what is actually happening in the environment. What the worker manager should be doing long term is comparing what we think we should have (based on roles, servers, zones, settings, providers, etc) and comparing that directly to what exists currently (unit files/pods).
It feels like the worker table is really a crutch because we couldn't check the process space for miq worker processes.
carbonin
on 24 Jul 2020
Related issues
ThomasBuchinger
路
6Comments
jawatts
路
8Comments
lpichler
路
6Comments
gpavinteractiv
路
8Comments
zhitongLBN
路
6Comments
Most helpful comment
Thinking about this more, it feels like this would be a good first step to removing the workers table (or at least its role in reconciliation). The problem we have here is that we're treating the table as "reality" when really we can inspect what is actually happening in the environment. What the worker manager should be doing long term is comparing what we think we should have (based on roles, servers, zones, settings, providers, etc) and comparing that directly to what exists currently (unit files/pods).
It feels like the worker table is really a crutch because we couldn't check the process space for miq worker processes.