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.

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

Review implementation of syncMachinesNodeTemplates function #334

Closed
ggaurav10 opened this issue Sep 13, 2019 · 2 comments
Closed

Review implementation of syncMachinesNodeTemplates function #334

ggaurav10 opened this issue Sep 13, 2019 · 2 comments
Labels
component/mcm Machine Controller Manager (including Node Problem Detector, Cluster Auto Scaler, etc.) kind/bug Bug lifecycle/rotten Nobody worked on this for 12 months (final aging stage) needs/planning Needs (more) planning with other MCM maintainers platform/all priority/3 Priority (lower number equals higher priority) size/s Size of pull request is small (see gardener-robot robot/bots/size.py)

Comments

@ggaurav10
Copy link
Contributor

ggaurav10 commented Sep 13, 2019

In syncMachinesNodeTemplates function, the copyMachineSetNodeTemplatesToMachines function updates the machine object directly in the cache (without deepcopy).

Ideally we should be using updateMachineWithRetries function to update the node template into the machine, which is currently a loop.

@ggaurav10
Copy link
Contributor Author

Also, this, this and this comments are not valid for MCM.
These comments seem to be taken directly from replicaset controller

@ghost ghost added lifecycle/stale Nobody worked on this for 6 months (will further age) and removed lifecycle/stale Nobody worked on this for 6 months (will further age) labels Nov 17, 2019
@ghost ghost added lifecycle/stale Nobody worked on this for 6 months (will further age) and removed lifecycle/stale Nobody worked on this for 6 months (will further age) labels Jan 17, 2020
@ghost ghost added component/mcm Machine Controller Manager (including Node Problem Detector, Cluster Auto Scaler, etc.) and removed component/machine-controller-manager labels Mar 7, 2020
@ghost ghost added the lifecycle/stale Nobody worked on this for 6 months (will further age) label May 7, 2020
@gardener-robot gardener-robot added lifecycle/rotten Nobody worked on this for 12 months (final aging stage) and removed lifecycle/stale Nobody worked on this for 6 months (will further age) labels Jul 7, 2020
@prashanth26 prashanth26 removed the lifecycle/rotten Nobody worked on this for 12 months (final aging stage) label Aug 13, 2020
@gardener-robot gardener-robot added the lifecycle/stale Nobody worked on this for 6 months (will further age) label Oct 13, 2020
@gardener-robot gardener-robot added lifecycle/rotten Nobody worked on this for 12 months (final aging stage) and removed lifecycle/stale Nobody worked on this for 6 months (will further age) labels Dec 13, 2020
@himanshu-kun himanshu-kun added size/s Size of pull request is small (see gardener-robot robot/bots/size.py) priority/4 Priority (lower number equals higher priority) and removed lifecycle/rotten Nobody worked on this for 12 months (final aging stage) labels Feb 14, 2023
@himanshu-kun himanshu-kun added priority/2 Priority (lower number equals higher priority) priority/3 Priority (lower number equals higher priority) needs/planning Needs (more) planning with other MCM maintainers and removed priority/4 Priority (lower number equals higher priority) priority/2 Priority (lower number equals higher priority) labels Feb 14, 2023
@gardener-robot gardener-robot added the lifecycle/stale Nobody worked on this for 6 months (will further age) label Oct 25, 2023
@gardener-robot gardener-robot added lifecycle/rotten Nobody worked on this for 12 months (final aging stage) and removed lifecycle/stale Nobody worked on this for 6 months (will further age) labels Jul 3, 2024
@aaronfern
Copy link
Contributor

aaronfern commented Jan 17, 2025

As per controller runtime best practices, a single reconciliation run should be quick, and reconciliation should be done via requeues and not retries
Will be tackled with #895

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/mcm Machine Controller Manager (including Node Problem Detector, Cluster Auto Scaler, etc.) kind/bug Bug lifecycle/rotten Nobody worked on this for 12 months (final aging stage) needs/planning Needs (more) planning with other MCM maintainers platform/all priority/3 Priority (lower number equals higher priority) size/s Size of pull request is small (see gardener-robot robot/bots/size.py)
Projects
None yet
Development

No branches or pull requests

7 participants