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

NET-6722 Rename MeshConfigController to ConsulResourceController #3283

Merged
merged 15 commits into from
Nov 30, 2023

Conversation

t-eckert
Copy link
Contributor

@t-eckert t-eckert commented Nov 29, 2023

The names MeshConfigController and MeshConfig were initially chosen to represent the generic Consul Resource mapping from Kubernetes into Consul.

Previously, these resources were referred to as "config-entries". With the upcoming implementation of Mesh Gateways V2, there was a name conflict with that implementation's MeshConfiguration resource and MeshConfigurationController.

It was decided that the existing generic MeshConfig usage be replaced with ConsulResource. This PR adopts that naming scheme.

This change is not user-facing.

Changes proposed in this PR:

  • Refactor MeshConfigController to ConsulResouceController
  • Rename controller file
  • Rename MeshConfig to ConsulResource
  • Rename Controller to ResourceController

How I've tested this PR:

  • Ensuring existing tests continue to pass.

How I expect reviewers to test this PR:

Checklist:

@t-eckert t-eckert added pr/no-changelog PR does not need a corresponding .changelog entry pr/no-backport signals that a PR will not contain a backport label labels Nov 29, 2023
@t-eckert t-eckert marked this pull request as ready for review November 29, 2023 20:13
@t-eckert t-eckert requested review from thisisnotashwin and a team November 29, 2023 20:13
Copy link
Contributor

@thisisnotashwin thisisnotashwin left a comment

Choose a reason for hiding this comment

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

This looks really good!! thanks for taking care of it so quick. i left a few comments inline.

@thisisnotashwin
Copy link
Contributor

looks like there are some linter errors that need cleaning up.. based on the cursory look, it seems fairly straightforward. let me know if you need hellp with those

Copy link
Contributor

@missylbytes missylbytes left a comment

Choose a reason for hiding this comment

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

LGTM

@t-eckert
Copy link
Contributor Author

t-eckert commented Nov 30, 2023

looks like there are some linter errors that need cleaning up.. based on the cursory look, it seems fairly straightforward. let me know if you need hellp with those

Yeah, it looks like an update to the retry linter caught some lingering issues.

The KubectlOptions function in the TestContext interface was written to use t *testing.T. That needs to be loosened to allow for the retry testing context.

EDIT: I'm going to fix this in another PR then rebase this PR.

EDIT 2: In the meantime, I loosened the restriction so others are unblocked.

@t-eckert t-eckert enabled auto-merge (squash) November 30, 2023 19:23
@t-eckert t-eckert merged commit fd0af5e into main Nov 30, 2023
3 checks passed
@t-eckert t-eckert deleted the NET-6722-rename-meshconfigctrlr branch November 30, 2023 19:32
sarahalsmiller pushed a commit that referenced this pull request Jan 5, 2024
* Refactor MeshConfigController to ConsulResouceController

* Rename controller file

* Rename MeshConfig to ConsulResource

* Rename Controller to ResourceController

* Update some references to meshConfig

* Fixup some comments

* Change MeshConfigController to ConsulResourceController in all implementations

* Change MeshConfigController to ConsulResourceController in instances

* Fix references to MeshConfigController in some tests

* Rename mockMeshConfig to mockConsulResource

* Rename meshConfigReconciler to consulResourceReconciler

* Rename ConsulResourceController to Controller

* Fix calls to validate

* Use "Controller" instead of "Reconciler"

* Empty commit to retrigger tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/no-backport signals that a PR will not contain a backport label 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