From e62926fd09df73605cc6b968371ccb8b30227bdd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20Burzy=C5=84ski?= Date: Wed, 24 May 2023 21:12:46 +0200 Subject: [PATCH 1/2] feat: add default-true FillIDs feature gate --- CHANGELOG.md | 11 ++++++++++- FEATURE_GATES.md | 1 + internal/dataplane/parser/parser.go | 11 +++++++++-- internal/dataplane/parser/parser_test.go | 13 ++++++++++++- internal/manager/featuregates/feature_gates.go | 5 +++++ 5 files changed, 37 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7fff2da40c..b5c7edf911 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -77,10 +77,19 @@ Adding a new version? You'll need three changes: [#3832](https://github.com/Kong/kubernetes-ingress-controller/pull/3832) - Added license agent for Konnect-managed instances. [#3883](https://github.com/Kong/kubernetes-ingress-controller/pull/3883) -- `Service`, `Route` and `Consumer` Kong entities now get assigned deterministic +- `Service`, `Route`, and `Consumer` Kong entities now get assigned deterministic IDs based on their unique properties (name, username, etc.) instead of random UUIDs. + After the upgrade to 2.10.0, the controller will re-create all the existing + entities (Services, Routes, and Consumers) with the new IDs. That can + potentially lead to temporary downtime between the deletion of the old entities + and the creation of the new ones. + Users might want to disable the feature if their existing DB-backed setup + consists of a huge amount of entities for which the recreation can take + significant time. It is possible to opt out of this behavior by setting + `FillIDs` feature gate to `false`. [#3933](https://github.com/Kong/kubernetes-ingress-controller/pull/3933) + [#4075](https://github.com/Kong/kubernetes-ingress-controller/pull/4075) - Added translator to translate ingresses under `networking.k8s.io/v1` to expression based Kong routes. The translator is enabled when feature gate `ExpressionRoutes` is turned on and the managed Kong gateway runs in router diff --git a/FEATURE_GATES.md b/FEATURE_GATES.md index 7390e532c7..2bdf597172 100644 --- a/FEATURE_GATES.md +++ b/FEATURE_GATES.md @@ -62,6 +62,7 @@ Features that reach GA and over time become stable will be removed from this tab | GatewayAlpha | `false` | Alpha | 2.6.0 | TBD | | ExpressionRoutes | `false` | Alpha | 2.10.0 | TBD | | CombinedServices | `false` | Alpha | 2.10.0 | TBD | +| FillIDs | `true` | Beta | 2.10.0 | 3.0.0 | **NOTE**: The `Gateway` feature gate refers to [Gateway API](https://github.com/kubernetes-sigs/gateway-api) APIs which are in diff --git a/internal/dataplane/parser/parser.go b/internal/dataplane/parser/parser.go index 486b11e24d..c0e3cf13cd 100644 --- a/internal/dataplane/parser/parser.go +++ b/internal/dataplane/parser/parser.go @@ -70,6 +70,10 @@ type FeatureFlags struct { // CombinedServices enables parser to create a single Kong Service when a Kubernetes Service is referenced // by multiple Ingresses. This is effective only when EnableCombinedServiceRoutes is enabled. CombinedServices bool + + // FillIDs enables the parser to fill in the IDs fields of Kong entities - Services, Routes, and Consumers - based + // on their names. It ensures that IDs remain stable across restarts of the controller. + FillIDs bool } func NewFeatureFlags( @@ -93,6 +97,7 @@ func NewFeatureFlags( RegexPathPrefix: kongVersion.MajorMinorOnly().GTE(versions.ExplicitRegexPathVersionCutoff), ExpressionRoutes: expressionRoutesEnabled, CombinedServices: combinedRoutesEnabled && featureGates.Enabled(featuregates.CombinedServicesFeature), + FillIDs: featureGates.Enabled(featuregates.FillIDsFeature), } } @@ -233,8 +238,10 @@ func (p *Parser) BuildKongConfig() KongConfigBuildingResult { result.Licenses = append(result.Licenses, p.licenseGetter.GetLicense()) } - // generate IDs for Kong entities - result.FillIDs(p.logger) + if p.featureFlags.FillIDs { + // generate IDs for Kong entities + result.FillIDs(p.logger) + } return KongConfigBuildingResult{ KongState: &result, diff --git a/internal/dataplane/parser/parser_test.go b/internal/dataplane/parser/parser_test.go index c8186a4a2c..3821a6a717 100644 --- a/internal/dataplane/parser/parser_test.go +++ b/internal/dataplane/parser/parser_test.go @@ -5321,6 +5321,15 @@ func TestNewFeatureFlags(t *testing.T) { CombinedServices: false, }, }, + { + name: "fill ids enabled", + featureGates: map[string]bool{ + featuregates.FillIDsFeature: true, + }, + expectedFeatureFlags: FeatureFlags{ + FillIDs: true, + }, + }, } for _, tc := range testCases { @@ -5341,7 +5350,9 @@ func TestNewFeatureFlags(t *testing.T) { } func mustNewParser(t *testing.T, storer store.Storer) *Parser { - p, err := NewParser(logrus.New(), storer, FeatureFlags{}) + p, err := NewParser(logrus.New(), storer, FeatureFlags{ + FillIDs: featuregates.GetFeatureGatesDefaults()[featuregates.FillIDsFeature], + }) require.NoError(t, err) return p } diff --git a/internal/manager/featuregates/feature_gates.go b/internal/manager/featuregates/feature_gates.go index 2504db6e17..155a5f40d6 100644 --- a/internal/manager/featuregates/feature_gates.go +++ b/internal/manager/featuregates/feature_gates.go @@ -37,6 +37,10 @@ const ( // a Kubernetes Service is referenced by multiple netv1.Ingresses. It's effective only when CombinedRoutesFeature is enabled. CombinedServicesFeature = "CombinedServices" + // FillIDsFeature is the name of the feature-gate that makes KIC fill in the ID fields of Kong entities (Services, + // Routes, and Consumers). It ensures that IDs remain stable across restarts of the controller. + FillIDsFeature = "FillIDs" + // DocsURL provides a link to the documentation for feature gates in the KIC repository. DocsURL = "https://github.com/Kong/kubernetes-ingress-controller/blob/main/FEATURE_GATES.md" ) @@ -78,5 +82,6 @@ func GetFeatureGatesDefaults() map[string]bool { CombinedRoutesFeature: true, ExpressionRoutesFeature: false, CombinedServicesFeature: false, + FillIDsFeature: true, } } From 2dd816822f664eaffc512ff6297cf2e589a4fdec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20Burzy=C5=84ski?= Date: Thu, 25 May 2023 15:58:35 +0200 Subject: [PATCH 2/2] address review comments --- .github/workflows/_integration_tests.yaml | 6 +++++ CHANGELOG.md | 22 +++++++++++-------- FEATURE_GATES.md | 2 +- internal/dataplane/parser/parser_test.go | 2 +- .../manager/featuregates/feature_gates.go | 2 +- 5 files changed, 22 insertions(+), 12 deletions(-) diff --git a/.github/workflows/_integration_tests.yaml b/.github/workflows/_integration_tests.yaml index 56e3e45d15..2a0c285c00 100644 --- a/.github/workflows/_integration_tests.yaml +++ b/.github/workflows/_integration_tests.yaml @@ -64,6 +64,12 @@ jobs: - name: dbless-combined-services test: dbless feature_gates: "GatewayAlpha=true,CombinedServices=true" + - name: dbless-fill-ids + test: dbless + feature_gates: "GatewayAlpha=true,FillIDs=true" + - name: postgres-fill-ids + test: postgres + feature_gates: "GatewayAlpha=true,FillIDs=true" steps: - uses: Kong/kong-license@master diff --git a/CHANGELOG.md b/CHANGELOG.md index b5c7edf911..52d7014027 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -77,17 +77,21 @@ Adding a new version? You'll need three changes: [#3832](https://github.com/Kong/kubernetes-ingress-controller/pull/3832) - Added license agent for Konnect-managed instances. [#3883](https://github.com/Kong/kubernetes-ingress-controller/pull/3883) -- `Service`, `Route`, and `Consumer` Kong entities now get assigned deterministic - IDs based on their unique properties (name, username, etc.) instead of random - UUIDs. - After the upgrade to 2.10.0, the controller will re-create all the existing - entities (Services, Routes, and Consumers) with the new IDs. That can +- `Service`, `Route`, and `Consumer` Kong entities now can get assigned + deterministic IDs based on their unique properties (name, username, etc.) + instead of random UUIDs. To enable this feature, set `FillIDs` feature gate + to `true`. + It's going to be useful in cases where stable IDs are needed across multiple + Kong Gateways managed by KIC (e.g. for the integration with Konnect and + reporting metrics that later can be aggregated across multiple instances based + on the entity's ID). + When `FillIDs` will be enabled, the controller will re-create all the existing + entities (Services, Routes, and Consumers) with the new IDs assigned. That can potentially lead to temporary downtime between the deletion of the old entities and the creation of the new ones. - Users might want to disable the feature if their existing DB-backed setup - consists of a huge amount of entities for which the recreation can take - significant time. It is possible to opt out of this behavior by setting - `FillIDs` feature gate to `false`. + Users should be cautious about enabling the feature if their existing DB-backed + setup consists of a huge amount of entities for which the recreation can take + significant time. [#3933](https://github.com/Kong/kubernetes-ingress-controller/pull/3933) [#4075](https://github.com/Kong/kubernetes-ingress-controller/pull/4075) - Added translator to translate ingresses under `networking.k8s.io/v1` to diff --git a/FEATURE_GATES.md b/FEATURE_GATES.md index 2bdf597172..7704177b0b 100644 --- a/FEATURE_GATES.md +++ b/FEATURE_GATES.md @@ -62,7 +62,7 @@ Features that reach GA and over time become stable will be removed from this tab | GatewayAlpha | `false` | Alpha | 2.6.0 | TBD | | ExpressionRoutes | `false` | Alpha | 2.10.0 | TBD | | CombinedServices | `false` | Alpha | 2.10.0 | TBD | -| FillIDs | `true` | Beta | 2.10.0 | 3.0.0 | +| FillIDs | `false` | Alpha | 2.10.0 | 3.0.0 | **NOTE**: The `Gateway` feature gate refers to [Gateway API](https://github.com/kubernetes-sigs/gateway-api) APIs which are in diff --git a/internal/dataplane/parser/parser_test.go b/internal/dataplane/parser/parser_test.go index 3821a6a717..98a4e333c3 100644 --- a/internal/dataplane/parser/parser_test.go +++ b/internal/dataplane/parser/parser_test.go @@ -5351,7 +5351,7 @@ func TestNewFeatureFlags(t *testing.T) { func mustNewParser(t *testing.T, storer store.Storer) *Parser { p, err := NewParser(logrus.New(), storer, FeatureFlags{ - FillIDs: featuregates.GetFeatureGatesDefaults()[featuregates.FillIDsFeature], + FillIDs: true, // We'll assume this is true for all tests. }) require.NoError(t, err) return p diff --git a/internal/manager/featuregates/feature_gates.go b/internal/manager/featuregates/feature_gates.go index 155a5f40d6..b34948fa2a 100644 --- a/internal/manager/featuregates/feature_gates.go +++ b/internal/manager/featuregates/feature_gates.go @@ -82,6 +82,6 @@ func GetFeatureGatesDefaults() map[string]bool { CombinedRoutesFeature: true, ExpressionRoutesFeature: false, CombinedServicesFeature: false, - FillIDsFeature: true, + FillIDsFeature: false, } }