Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Allow disabling of module having errors (during it's installation or in hooks) #43

Closed
dmgtn opened this issue Aug 21, 2019 · 1 comment · Fixed by #315
Closed

Allow disabling of module having errors (during it's installation or in hooks) #43

dmgtn opened this issue Aug 21, 2019 · 1 comment · Fixed by #315

Comments

@dmgtn
Copy link
Member

dmgtn commented Aug 21, 2019

The current implementation has possible deadlock: if the module has errors, either in the helm templates or in hooks, the tasks are stuck in the queue and disabling the module in ConfigMap don't help, because of module tasks are not removed from the queue.

Proposed change: When the module becomes disabled (in ConfigMap, or by any other means) the following should happen (in the following order):

  1. All informers should be stopped and all crontab bindings should be disabled, so no new tasks (possibly containing errors) will be added to the queue.
  2. All existing module tasks should be removed from the queue.
  3. Tasks associated with module disabling should be added (helm uninstall, and afterDeleteHelm hooks)
@diafour diafour mentioned this issue Sep 20, 2019
Open
@diafour
Copy link
Contributor

diafour commented Oct 9, 2019

Error in enabled script can also cause this situation. #16

@diafour diafour mentioned this issue Oct 9, 2019
Closed
@diafour diafour added this to the 1.0.0-beta.7 milestone Apr 3, 2020
@diafour diafour removed this from the 1.0.0-beta.7 milestone Jun 26, 2020
diafour added a commit that referenced this issue Apr 7, 2022
@diafour
fix: more responsive ConfigMap changes
43269ba 
Huge refactoring to speedup reactions to ConfigMap changes. Fixes in enabled state
calculations.

KubeConfigManager is now responsible to signal if config is changed or invalid.

- fix: Update metric if ConfigMap is invalid (malformed module name or module values YAML)

ModuleManager is now the only component that runs enabled scripts. It is responsible
for calculating ModulesReload/ReloadAll event. It lists Helm releases once at start
to detect initial enabled modules and modules to purge.

- fix: Fresh config values for enabled scripts (#184, #16)

AddonOperator uses task ConvergeModules to handle changes in ConfigMap or global values
and reload all modules or only changed.

- fix: ModulePurge tasks moved between GlobalSynchronization and first ConvergeModules (#233)
- fix: Clear queues from tasks for disabled module or errored hooks on ConfigMap changes (#43)
- ref: ConvergeModules is a new multi-phase task.

Other:
- mocks for HelmResourcesManager and KubeConfigManager
- Helm struct instead of global variables helm.NewClient and helm.HealthzHandler
-
@diafour diafour mentioned this issue Apr 7, 2022
Merged
3 tasks
diafour added a commit that referenced this issue Apr 25, 2022
@diafour
fix: more responsive ConfigMap changes
d2e392e 
Huge refactoring to speedup reactions to ConfigMap changes. Fixes in enabled state
calculations.

KubeConfigManager is now responsible to signal if config is changed or invalid.

- fix: Update metric if ConfigMap is invalid (malformed module name or module values YAML)
- use runtime config to enable debug
- add locking

ModuleManager is now the only component that runs enabled scripts. It is responsible
for calculating ModulesReload/ReloadAll event. It lists Helm releases once at start
to detect initial enabled modules and modules to purge.

- fix: Fresh config values for enabled scripts (#184, #16)

AddonOperator uses task ConvergeModules to handle changes in ConfigMap or global values
and reload all modules or only changed.

- fix: ModulePurge tasks moved between GlobalSynchronization and first ConvergeModules (#233)
- fix: Clear queues from tasks for disabled module or errored hooks on ConfigMap changes (#43)
- ref: ConvergeModules is a new multi-phase task.

Other:
- mocks for HelmResourcesManager and KubeConfigManager
- Helm struct instead of global variables helm.NewClient and helm.HealthzHandler
- by dependabot: update gomega
diafour added a commit that referenced this issue May 25, 2022
@diafour
fix: faster response to config map changes (#315)
930fc4e 
* fix: more responsive ConfigMap changes

Huge refactoring to speedup reactions to ConfigMap changes. Fixes in enabled state
calculations.

KubeConfigManager is now responsible to signal if config is changed or invalid.

- fix: Update metric if ConfigMap is invalid (malformed module name or module values YAML)
- use runtime config to enable debug
- add locking

ModuleManager is now the only component that runs enabled scripts. It is responsible
for calculating ModulesReload/ReloadAll event. It lists Helm releases once at start
to detect initial enabled modules and modules to purge.

- fix: Fresh config values for enabled scripts (#184, #16)

AddonOperator uses task ConvergeModules to handle changes in ConfigMap or global values
and reload all modules or only changed.

- fix: ModulePurge tasks moved between GlobalSynchronization and first ConvergeModules (#233)
- fix: Clear queues from tasks for disabled module or errored hooks on ConfigMap changes (#43)
- ref: ConvergeModules is a new multi-phase task.

Other:
- mocks for HelmResourcesManager and KubeConfigManager
- Helm struct instead of global variables helm.NewClient and helm.HealthzHandler
- by dependabot: update gomega
- reduce verbosity for info log level

Signed-off-by: Ivan Mikheykin <ivan.mikheykin@flant.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: faster response to config map changes flant/addon-operator
2 participants
@diafour @dmgtn