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

mesh: add computed destinations with a controller that computes them #19067

Merged
merged 10 commits into from
Oct 12, 2023

Conversation

ishustava
Copy link
Contributor

Description

This PR adds a new type ComputedDestinations that will contain all destinations from any Destinations resources and will be name-aligned with a workload. This also adds an explicit-destinations controller that computes these resources.

This is needed to simplify the tracking we need to do currently in the sidecar-proxy controller and makes it easier to query all explicit destinations that apply to a workload.

PR Checklist

  • updated test coverage
  • external facing docs updated
  • appropriate backport labels added
  • not a security concern

@ishustava ishustava changed the base branch from main to ishustava/NET-3995-computed-proxy-config October 4, 2023 21:14
@ishustava ishustava added pr/no-changelog PR does not need a corresponding .changelog entry pr/no-backport labels Oct 4, 2023
@ishustava ishustava force-pushed the ishustava/NET-3996-computed-destinations branch from a800d12 to 9d77602 Compare October 4, 2023 21:40
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: BUSL-1.1

package workloadselectionmapper
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I generalized this mapper as I needed the same functionality for both explicit destinations and proxy config controllers.

@ishustava ishustava force-pushed the ishustava/NET-3996-computed-destinations branch 3 times, most recently from 4025072 to e2184f8 Compare October 4, 2023 23:36
@ishustava ishustava force-pushed the ishustava/NET-3995-computed-proxy-config branch 3 times, most recently from d3e50af to 688a2eb Compare October 5, 2023 14:54
@ishustava ishustava force-pushed the ishustava/NET-3996-computed-destinations branch from e2184f8 to b11c7cb Compare October 5, 2023 15:49
@ishustava ishustava marked this pull request as ready for review October 5, 2023 16:31
@ishustava ishustava requested a review from a team as a code owner October 5, 2023 16:31
@ishustava ishustava requested a review from rboyer October 5, 2023 16:31
@ishustava ishustava force-pushed the ishustava/NET-3996-computed-destinations branch from b11c7cb to 68770d3 Compare October 6, 2023 22:25
Copy link
Member

@rboyer rboyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a few remaining comments.

Base automatically changed from ishustava/NET-3995-computed-proxy-config to main October 10, 2023 23:34
@ishustava ishustava force-pushed the ishustava/NET-3996-computed-destinations branch 2 times, most recently from 1cb0ef5 to 355c141 Compare October 11, 2023 23:59
serviceRefMapper: bimapper.New(pbmesh.ComputedExplicitDestinationsType, pbcatalog.ServiceType),
}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The methods below are tested in controller tests

@ishustava ishustava force-pushed the ishustava/NET-3996-computed-destinations branch from a44fdd2 to 24e59d6 Compare October 12, 2023 15:00
@ishustava ishustava merged commit ad06c96 into main Oct 12, 2023
@ishustava ishustava deleted the ishustava/NET-3996-computed-destinations branch October 12, 2023 18:04
ishustava added a commit that referenced this pull request Oct 12, 2023
This change builds on #19043 and #19067 and updates the sidecar controller to use those computed resources. This achieves several benefits:

   * The cache is now simplified which helps us solve for previous bugs (such as multiple Upstreams/Destinations targeting the same service would overwrite each other)
   * We no longer need proxy config cache
   * We no longer need to do merging of proxy configs as part of the controller logic
   * Controller watches are simplified because we no longer need to have complex mapping using cache and can instead use the simple ReplaceType mapper.

It also makes several other improvements/refactors:

  * Unifies all caches into one. This is because originally the caches were more independent, however, now that they need to interact with each other it made sense to unify them where sidecar proxy controller uses one cache with 3 bimappers
   * Unifies cache and mappers. Mapper already needed all caches anyway and so it made sense to make the cache do the mapping also now that the cache is unified.
   * Gets rid of service endpoints watches. This was needed to get updates in a case when service's identities have changed and we need to update proxy state template's spiffe IDs for those destinations. This will however generate a lot of reconcile requests for this controller as service endpoints objects can change a lot because they contain workload's health status. This is solved by adding a status to the service object tracking "bound identities" and have service endpoints controller update it. Having service's status updated allows us to get updates in the sidecar proxy controller because it's already watching service objects
   * Add a watch for workloads. We need it so that we get updates if workload's ports change. This also ensures that we update cached identities in case workload's identity changes.
@ishustava ishustava added backport/1.17 This release series is no longer active on CE. Use backport/ent/1.17. and removed pr/no-backport labels Oct 13, 2023
ishustava added a commit that referenced this pull request Oct 13, 2023
…19067)

This commit adds a new type ComputedDestinations that will contain all destinations from any Destinations resources and will be name-aligned with a workload. This also adds an explicit-destinations controller that computes these resources.

This is needed to simplify the tracking we need to do currently in the sidecar-proxy controller and makes it easier to query all explicit destinations that apply to a workload.
ishustava added a commit that referenced this pull request Oct 13, 2023
…mputes them into release/1.17.x (#19194)

mesh: add computed destinations with a controller that computes them (#19067)

This commit adds a new type ComputedDestinations that will contain all destinations from any Destinations resources and will be name-aligned with a workload. This also adds an explicit-destinations controller that computes these resources.

This is needed to simplify the tracking we need to do currently in the sidecar-proxy controller and makes it easier to query all explicit destinations that apply to a workload.

Co-authored-by: Iryna Shustava <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.17 This release series is no longer active on CE. Use backport/ent/1.17. pr/no-changelog PR does not need a corresponding .changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants