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

ca: clearly identify methods that are primary-only or secondary-only #11338

Merged
merged 3 commits into from
Nov 1, 2021

Conversation

dnephin
Copy link
Contributor

@dnephin dnephin commented Oct 15, 2021

Best viewed by individual commit.

The CAMananger has two distinct "modes". It can operator as a primary (in the primary DC), or a secondary (in a secondary DC). Some of the methods on CAManager are clearly identified as such, but not all.

This PR renames all methods that are primary-only or secondary-only to start with a primary or secondary prefix. This is one small step to help make these two "modes" more obvious, which will help prevent issues like the one described in #11159. In the future we may be able to make these two modes more concrete in code by having separate types for each.

Every other method uses c not ca
This commit renames functions to use a consistent pattern for identifying the functions that
can only be called when the Manager is run as the primary or secondary.

This is a step toward eventually creating separate types and moving these methods off of CAManager.
This function is only run when the CAManager is a primary. Extracting this function
makes it clear which parts of UpdateConfiguration are run only in the primary and
also makes the cleanup logic simpler. Instead of both a defer and a local var we
can call the cleanup function in two places.
@dnephin dnephin added theme/internal-cleanup Used to identify tech debt, testing improvements, code refactoring, and non-impactful optimization theme/certificates Related to creating, distributing, and rotating certificates in Consul pr/no-changelog PR does not need a corresponding .changelog entry labels Oct 15, 2021
@dnephin dnephin requested a review from a team October 15, 2021 23:00
@github-actions github-actions bot removed the theme/certificates Related to creating, distributing, and rotating certificates in Consul label Oct 15, 2021
Copy link
Contributor

@kisunji kisunji left a comment

Choose a reason for hiding this comment

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

👍

@dnephin dnephin merged commit eee598e into main Nov 1, 2021
@dnephin dnephin deleted the dnephin/ca-manager-isolate-secondary branch November 1, 2021 18:10
@hc-github-team-consul-core
Copy link
Collaborator

🍒 If backport labels were added before merging, cherry-picking will start automatically.

To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/490362.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/no-changelog PR does not need a corresponding .changelog entry theme/internal-cleanup Used to identify tech debt, testing improvements, code refactoring, and non-impactful optimization
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants