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

Refactor Status Updater to be resource agnostic #1071

Closed
kate-osborn opened this issue Sep 18, 2023 · 1 comment
Closed

Refactor Status Updater to be resource agnostic #1071

kate-osborn opened this issue Sep 18, 2023 · 1 comment
Assignees
@pleshakov
Labels
refined Requirements are refined and the issue is ready to be implemented. size/large Estimated to be completed within two weeks tech-debt Short-term pain, long-term benefit
Milestone
v1.3.0

Comments

@kate-osborn
Copy link
Contributor

kate-osborn commented Sep 18, 2023 •
edited by mpstefan
Loading

Currently, the status updater is aware of what resource it is updating the status of. This makes it difficult to extend because for each new resource we add support for, we have to modify the status updater.

See this comment thread for more context.

A/C:

  • the status updater is not aware of which resource it is updating the status of
@kate-osborn kate-osborn added the tech-debt Short-term pain, long-term benefit label Sep 18, 2023
@kate-osborn kate-osborn mentioned this issue Sep 18, 2023
Merged
6 tasks
@mpstefan mpstefan added this to the v1.2.0 milestone Nov 30, 2023
@mpstefan mpstefan modified the milestones: v1.2.0, v2.0.0 Feb 2, 2024
@kate-osborn
Copy link
Contributor Author

@mpstefan I think this is high-priority tech-debt since adding policies will further strain our status updater component.

@mpstefan mpstefan added the backlog Currently unprioritized work. May change with user feedback or as the product progresses. label Mar 13, 2024
@mpstefan mpstefan modified the milestone: v1.3.0 Mar 13, 2024
@mpstefan mpstefan added size/large Estimated to be completed within two weeks refined Requirements are refined and the issue is ready to be implemented. and removed backlog Currently unprioritized work. May change with user feedback or as the product progresses. labels Mar 20, 2024
@pleshakov pleshakov self-assigned this Mar 22, 2024
@pleshakov pleshakov moved this from 🆕 New to 🏗 In Progress in NGINX Gateway Fabric Mar 22, 2024
pleshakov added a commit to pleshakov/nginx-gateway-fabric that referenced this issue Mar 28, 2024
@pleshakov
POC - Refactor Status Updater to be resource agnostic
2804d1f 
Problem:
Currently, the status updater is aware of what resource it is updating
the status of. This makes it difficult to extend because for each new
resource we add support for, we have to modify the status updater.

Solution:
Replace old status updater with two new ones

(1) Regular Updater, to update statuses of multiple resources via
UpdateRequest type

(2) CachingGroupUpdater to update statuses of groups of resources and
cache results until the updater is enabled (when pod becomes leader).
It uses Regular Updater.

Using groups allow replacing statuses of subset of resources, without
the need to recompute all cached statuses.

The new status2 package (which will replace status package) is
resource agnostic. This is acomplished by representing status update
as a function (Setter).

The manager packages were updated:
- provisioner mode, to use regular Updater
- static mode, to use CachingGroupUpdater

status Setters were updated and moved to the static package.

CLOSES -- nginxinc#1071
@pleshakov pleshakov mentioned this issue Mar 28, 2024
Closed
6 tasks
pleshakov added a commit to pleshakov/nginx-gateway-fabric that referenced this issue Apr 5, 2024
@pleshakov
POC - Refactor Status Updater to be resource agnostic
91fdc0e 
Problem:
Currently, the status updater is aware of what resource it is updating
the status of. This makes it difficult to extend because for each new
resource we add support for, we have to modify the status updater.

Solution:
Replace old status updater with two new ones

(1) Regular Updater, to update statuses of multiple resources via
UpdateRequest type

(2) CachingGroupUpdater to update statuses of groups of resources and
cache results until the updater is enabled (when pod becomes leader).
It uses Regular Updater.

Using groups allow replacing statuses of subset of resources, without
the need to recompute all cached statuses.

The new status2 package (which will replace status package) is
resource agnostic. This is acomplished by representing status update
as a function (Setter).

The manager packages were updated:
- provisioner mode, to use regular Updater
- static mode, to use CachingGroupUpdater

status Setters were updated and moved to the static package.

CLOSES -- nginxinc#1071
pleshakov added a commit to pleshakov/nginx-gateway-fabric that referenced this issue Apr 5, 2024
@pleshakov
Refactor Status Updater to be resource agnostic
6befd1e 
Problem:
Currently, the status updater is aware of what resource it is updating
the status of. This makes it difficult to extend because for each new
resource we add support for, we have to modify the status updater.

Solution:

Replace old status updater with two new ones

(1) Regular Updater, to update statuses of multiple resources via
UpdateRequest type.

(2) LeaderAwareGroupUpdater to update statuses of groups of resources
and save any pending UpdateRequests until the updater is enabled
(when pod becomes leader). It uses Regular Updater.

Using groups allow replacing UpdateRequests of subset of resources,
without the need to recompute UpdateRequests for all resources.

The new status package is
resource agnostic. This is accomplished by representing status update
as a function (Setter).

Other updates:
- provisioner manager uses regular Updater.
- static manager uses LeaderAwareGroupUpdater (because it needs to
  support leader election)
- framework Status setter were simplified and moved to static/status
  package
- status related functions in static package were moved to
  static/status package.
- Previous Build* functions were replaced with PrepareRequests
  functions. Such functions don't create any intermediate status
  representation (like it was before), but create status UpdateRequests
  which directly set the resource statuses.

CLOSES -- nginxinc#1071
@pleshakov pleshakov mentioned this issue Apr 5, 2024
Merged
6 tasks
@pleshakov pleshakov moved this from 🏗 In Progress to 👀 In Review in NGINX Gateway Fabric Apr 5, 2024
pleshakov added a commit that referenced this issue Apr 16, 2024
@pleshakov
Refactor Status Updater to be resource agnostic (#1814)
2277050 
Problem:
Currently, the status updater is aware of what resource it is updating
the status of. This makes it difficult to extend because for each new
resource we add support for, we have to modify the status updater.

Solution:

Replace old status updater with two new ones

(1) Regular Updater, to update statuses of multiple resources via
UpdateRequest type.

(2) LeaderAwareGroupUpdater to update statuses of groups of resources
and save any pending UpdateRequests until the updater is enabled
(when pod becomes leader). It uses Regular Updater.

Using groups allow replacing UpdateRequests of subset of resources,
without the need to recompute UpdateRequests for all resources.

The new status package is
resource agnostic. This is accomplished by representing status update
as a function (Setter).

Other updates:
- provisioner manager uses regular Updater.
- static manager uses LeaderAwareGroupUpdater (because it needs to
  support leader election)
- framework Status setter were simplified and moved to static/status
  package
- status related functions in static package were moved to
  static/status package.
- Previous Build* functions were replaced with PrepareRequests
  functions. Such functions don't create any intermediate status
  representation (like it was before), but create status UpdateRequests
  which directly set the resource statuses.
- static manager handler conditionally updates statuses GatewayClass
  based on manager configuration. Previously, the conditional logic was
  implemented in the Status Updater.

CLOSES -- #1071
@github-project-automation github-project-automation bot moved this from 👀 In Review to ✅ Done in NGINX Gateway Fabric Apr 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@pleshakov pleshakov

Labels
refined Requirements are refined and the issue is ready to be implemented. size/large Estimated to be completed within two weeks tech-debt Short-term pain, long-term benefit
Projects
NGINX Gateway Fabric
Archived in project
Milestone
v1.3.0
Development

No branches or pull requests
3 participants
@pleshakov @mpstefan @kate-osborn