Skip to content

Commit

Permalink
No direct CRD labels; direct will be the expected default
Browse files Browse the repository at this point in the history
  • Loading branch information
yuwenma committed Jun 3, 2024
1 parent 3e70af2 commit 9a749a4
Show file tree
Hide file tree
Showing 12 changed files with 128 additions and 30 deletions.
11 changes: 11 additions & 0 deletions .github/workflows/presubmit.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,17 @@ jobs:
- name: "Run pause tests"
run: |
./scripts/github-actions/ga-pause-test.sh
direct-tests:
runs-on: ubuntu-22.04
timeout-minutes: 60
steps:
- uses: actions/checkout@v3
- uses: actions/setup-go@v4
with:
go-version-file: 'go.mod'
- name: "run tests-e2e-direct"
run: |
./scripts/github-actions/tests-e2e-direct.sh
tests-e2e-fixtures-vcr:
runs-on: ubuntu-22.04
timeout-minutes: 60
Expand Down
2 changes: 1 addition & 1 deletion apis/apikeys/v1alpha1/apikey_type.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ var (
// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
// +kubebuilder:resource:categories=gcp,shortName=gcpapikeyskey;gcpapikeyskeys
// +kubebuilder:subresource:status
// +kubebuilder:metadata:labels="cnrm.cloud.google.com/directcrd=true";"cnrm.cloud.google.com/managed-by-kcc=true";"cnrm.cloud.google.com/stability-level=alpha";"cnrm.cloud.google.com/system=true"
// +kubebuilder:metadata:labels="cnrm.cloud.google.com/tf2crd=true";"cnrm.cloud.google.com/managed-by-kcc=true";"cnrm.cloud.google.com/stability-level=alpha";"cnrm.cloud.google.com/system=true"
// +kubebuilder:printcolumn:name="Age",JSONPath=".metadata.creationTimestamp",type="date"
// +kubebuilder:printcolumn:name="Ready",JSONPath=".status.conditions[?(@.type=='Ready')].status",type="string",description="When 'True', the most recent reconcile of the resource succeeded"
// +kubebuilder:printcolumn:name="Status",JSONPath=".status.conditions[?(@.type=='Ready')].reason",type="string",description="The reason for the value in 'Ready'"
Expand Down
2 changes: 1 addition & 1 deletion apis/resources/logging/v1beta1/logmetric_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ type LoggingLogMetricStatus struct {
// +kubebuilder:subresource:status
// +kubebuilder:object:root=true
// +kubebuilder:subresource:status
// +kubebuilder:metadata:labels="cnrm.cloud.google.com/directcrd=true";"cnrm.cloud.google.com/managed-by-kcc=true";"cnrm.cloud.google.com/stability-level=stable";"cnrm.cloud.google.com/system=true"
// +kubebuilder:metadata:labels="cnrm.cloud.google.com/managed-by-kcc=true";"cnrm.cloud.google.com/stability-level=stable";"cnrm.cloud.google.com/system=true"
// +kubebuilder:printcolumn:name="Age",JSONPath=".metadata.creationTimestamp",type="date"
// +kubebuilder:printcolumn:name="Ready",JSONPath=".status.conditions[?(@.type=='Ready')].status",type="string",description="When 'True', the most recent reconcile of the resource succeeded"
// +kubebuilder:printcolumn:name="Status",JSONPath=".status.conditions[?(@.type=='Ready')].reason",type="string",description="The reason for the value in 'Ready'"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ metadata:
cnrm.cloud.google.com/version: 0.0.0-dev
creationTimestamp: null
labels:
cnrm.cloud.google.com/directcrd: "true"
cnrm.cloud.google.com/managed-by-kcc: "true"
cnrm.cloud.google.com/stability-level: alpha
cnrm.cloud.google.com/system: "true"
cnrm.cloud.google.com/tf2crd: "true"
name: apikeyskeys.apikeys.cnrm.cloud.google.com
spec:
group: apikeys.cnrm.cloud.google.com
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ metadata:
cnrm.cloud.google.com/version: 0.0.0-dev
creationTimestamp: null
labels:
cnrm.cloud.google.com/directcrd: "true"
cnrm.cloud.google.com/managed-by-kcc: "true"
cnrm.cloud.google.com/stability-level: stable
cnrm.cloud.google.com/system: "true"
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

45 changes: 43 additions & 2 deletions pkg/controller/direct/directbase/directbase_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (
"github.com/GoogleCloudPlatform/k8s-config-connector/pkg/execution"
"github.com/GoogleCloudPlatform/k8s-config-connector/pkg/k8s"
"github.com/GoogleCloudPlatform/k8s-config-connector/pkg/util"
apiextensions "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"

"golang.org/x/sync/semaphore"
apierrors "k8s.io/apimachinery/pkg/api/errors"
Expand All @@ -57,18 +58,21 @@ import (
var ControllerBuilder directControllerBuilder

func init() {
ControllerBuilder = directControllerBuilder{modelMapper: map[schema.GroupVersionKind]func(*controller.Config) Model{}}
ControllerBuilder = directControllerBuilder{}
}

type directControllerBuilder struct {
modelMapper map[schema.GroupVersionKind]func(*controller.Config) Model
}

func (c *directControllerBuilder) RegisterModel(gvk schema.GroupVersionKind, modelFn func(*controller.Config) Model) {
if c.modelMapper == nil {
c.modelMapper = map[schema.GroupVersionKind]func(*controller.Config) Model{}
}
c.modelMapper[gvk] = modelFn
}

func (c *directControllerBuilder) Add(mgr manager.Manager, config *controller.Config, gvk schema.GroupVersionKind, deps Deps) error {
func (c *directControllerBuilder) AddController(mgr manager.Manager, config *controller.Config, gvk schema.GroupVersionKind, deps Deps) error {
immediateReconcileRequests := make(chan event.GenericEvent, k8s.ImmediateReconcileRequestsBufferSize)
resourceWatcherRoutines := semaphore.NewWeighted(k8s.MaxNumResourceWatcherRoutines)

Expand All @@ -79,6 +83,43 @@ func (c *directControllerBuilder) Add(mgr manager.Manager, config *controller.Co
return add(mgr, reconciler)
}

func (c *directControllerBuilder) IsDirectByCRD(crd *apiextensions.CustomResourceDefinition) bool {
for gvk, _ := range c.modelMapper {
if gvk.Group == crd.Spec.Group && gvk.Kind == crd.Spec.Names.Kind {
for _, version := range crd.Spec.Versions {
if gvk.Version == version.Name {
return true
}
}

}
}
return false
}

func (c *directControllerBuilder) IsDirectByGK(gk schema.GroupKind) bool {
if c.modelMapper == nil {
return false
}
for gvk, _ := range c.modelMapper {
if gvk.Group == gk.Group && gvk.Kind == gk.Kind {
return true
}
}
return false
}

func (c *directControllerBuilder) IsDirectByGVK(gvk schema.GroupVersionKind) bool {
if c.modelMapper == nil {
return false
}
_, ok := c.modelMapper[gvk]
if ok {
return true
}
return false
}

// NewReconciler returns a new reconcile.Reconciler.
func (c *directControllerBuilder) NewReconciler(mgr manager.Manager, config *controller.Config, immediateReconcileRequests chan event.GenericEvent, resourceWatcherRoutines *semaphore.Weighted,
gvk schema.GroupVersionKind, jg jitter.Generator) (*DirectReconciler, error) {
Expand Down
41 changes: 21 additions & 20 deletions pkg/controller/registration/registration_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ func registerDefaultController(r *ReconcileRegistration, config *controller.Conf
}
var schemaUpdater k8s.SchemaReferenceUpdater
if kccfeatureflags.UseDirectReconciler(gvk.GroupKind()) {
err := directbase.ControllerBuilder.Add(r.mgr, config, gvk, directbase.Deps{JitterGenerator: r.jitterGenerator})
err := directbase.ControllerBuilder.AddController(r.mgr, config, gvk, directbase.Deps{JitterGenerator: r.jitterGenerator})
if err != nil {
return nil, fmt.Errorf("error adding direct controller for %v to a manager: %w", crd.Spec.Names.Kind, err)
}
Expand All @@ -221,14 +221,14 @@ func registerDefaultController(r *ReconcileRegistration, config *controller.Conf
}

default:
// register controllers for direct CRDs
if val, ok := crd.Labels[k8s.DirectCRDLabel]; ok && val == "true" {
err := directbase.ControllerBuilder.Add(r.mgr, config, gvk, directbase.Deps{JitterGenerator: r.jitterGenerator})
if err != nil {
return nil, fmt.Errorf("error adding direct controller for %v to a manager: %w", crd.Spec.Names.Kind, err)
// register the controller to automatically create secrets for GSA keys
if isServiceAccountKeyCRD(crd) {
logger.Info("registering the GSA-Key-to-Secret generation controller")
if err := gsakeysecretgenerator.Add(r.mgr, crd, &controller.Deps{JitterGen: r.jitterGenerator}); err != nil {
return nil, fmt.Errorf("error adding the gsa-to-secret generator for %v to a manager: %w", crd.Spec.Names.Kind, err)
}
return schemaUpdater, nil
}

// register controllers for dcl-based CRDs
if val, ok := crd.Labels[k8s.DCL2CRDLabel]; ok && val == "true" {
su, err := dclcontroller.Add(r.mgr, crd, r.dclConverter, r.dclConfig, r.smLoader, r.defaulters, r.jitterGenerator)
Expand All @@ -238,22 +238,23 @@ func registerDefaultController(r *ReconcileRegistration, config *controller.Conf
return su, nil
}
// register controllers for tf-based CRDs
if val, ok := crd.Labels[crdgeneration.TF2CRDLabel]; !ok || val != "true" {
logger.Error(fmt.Errorf("unrecognized CRD: %v", crd.Spec.Names.Kind), "skipping controller registration", "group", gvk.Group, "version", gvk.Version, "kind", gvk.Kind)
return nil, nil
}
su, err := tf.Add(r.mgr, crd, r.provider, r.smLoader, r.defaulters, r.jitterGenerator)
if err != nil {
return nil, fmt.Errorf("error adding terraform controller for %v to a manager: %w", crd.Spec.Names.Kind, err)
if val, ok := crd.Labels[crdgeneration.TF2CRDLabel]; ok && val == "true" {
su, err := tf.Add(r.mgr, crd, r.provider, r.smLoader, r.defaulters, r.jitterGenerator)
if err != nil {
return nil, fmt.Errorf("error adding terraform controller for %v to a manager: %w", crd.Spec.Names.Kind, err)
}
return su, nil
}
schemaUpdater = su
// register the controller to automatically create secrets for GSA keys
if isServiceAccountKeyCRD(crd) {
logger.Info("registering the GSA-Key-to-Secret generation controller")
if err := gsakeysecretgenerator.Add(r.mgr, crd, &controller.Deps{JitterGen: r.jitterGenerator}); err != nil {
return nil, fmt.Errorf("error adding the gsa-to-secret generator for %v to a manager: %w", crd.Spec.Names.Kind, err)
// register controllers for direct CRDs
if directbase.ControllerBuilder.IsDirectByGVK(gvk) {
err := directbase.ControllerBuilder.AddController(r.mgr, config, gvk, directbase.Deps{JitterGenerator: r.jitterGenerator})
if err != nil {
return nil, fmt.Errorf("error adding direct controller for %v to a manager: %w", crd.Spec.Names.Kind, err)
}
return schemaUpdater, nil
}
logger.Error(fmt.Errorf("unrecognized CRD: %v", crd.Spec.Names.Kind), "skipping controller registration", "group", gvk.Group, "version", gvk.Version, "kind", gvk.Kind)
return nil, nil
}
return schemaUpdater, nil
}
Expand Down
1 change: 0 additions & 1 deletion pkg/k8s/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,6 @@ var (
KCCVersionLabel = FormatAnnotation("version")
ScopedNamespaceLabel = FormatAnnotation("scoped-namespace")
DCL2CRDLabel = FormatAnnotation("dcl2crd")
DirectCRDLabel = FormatAnnotation("directcrd")
KCCStabilityLabel = FormatAnnotation("stability-level")

MutableButUnreadableFieldsAnnotation = FormatAnnotation("mutable-but-unreadable-fields")
Expand Down
2 changes: 1 addition & 1 deletion pkg/test/controller/reconciler/testreconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ func (r *TestReconciler) newReconcilerForCRD(crd *apiextensions.CustomResourceDe
if crd.GetLabels()[k8s.DCL2CRDLabel] == "true" {
return dclcontroller.NewReconciler(r.mgr, crd, r.dclConverter, r.dclConfig, r.smLoader, immediateReconcileRequests, resourceWatcherRoutines, defaulters, jg)
}
if crd.GetLabels()[k8s.DirectCRDLabel] == "true" {
if directbase.ControllerBuilder.IsDirectByCRD(crd) {
return directbase.ControllerBuilder.NewReconciler(r.mgr, &kcccontroller.Config{HTTPClient: r.httpClient}, immediateReconcileRequests, resourceWatcherRoutines, crd.GroupVersionKind(), jg)
}
}
Expand Down
47 changes: 47 additions & 0 deletions scripts/github-actions/tests-e2e-direct.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
#!/bin/bash
# Copyright 2024 Google LLC
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

set -o errexit
set -o nounset
set -o pipefail

REPO_ROOT="$(git rev-parse --show-toplevel)"

cd ${REPO_ROOT}/

echo "Downloading envtest assets..."
export KUBEBUILDER_ASSETS=$(go run sigs.k8s.io/controller-runtime/tools/setup-envtest@latest use -p path)

echo "Running e2e tests samples for LoggingLogMetric direct reconciliation..."

KCC_USE_DIRECT_RECONCILERS=LoggingLogMetric \
GOLDEN_OBJECT_CHECKS=1 \
GOLDEN_REQUEST_CHECKS=1 \
E2E_KUBE_TARGET=envtest RUN_E2E=1 E2E_GCP_TARGET=mock \
go test -test.count=1 -timeout 600s -v ./tests/e2e -run 'TestAllInSeries/samples/linear-log-metric|TestAllInSeries/samples/exponential-log-metric|TestAllInSeries/samples/int-log-metric|TestAllInSeries/samples/explicit-log-metric'

echo "Running e2e tests fixtures for LoggingLogMetric direct reconciliation..."

KCC_USE_DIRECT_RECONCILERS=LoggingLogMetric \
GOLDEN_OBJECT_CHECKS=1 \
GOLDEN_REQUEST_CHECKS=1 \
E2E_KUBE_TARGET=envtest RUN_E2E=1 E2E_GCP_TARGET=mock \
go test -test.count=1 -timeout 600s -v ./tests/e2e -run 'TestAllInSeries/fixtures/explicitlogmetric|TestAllInSeries/fixtures/exponentiallogmetric|TestAllInSeries/fixtures/linearlogmetric|TestAllInSeries/fixtures/logbucketmetric'

echo "Running scenarios tests for LoggingLogMetric direct reconciliation..."

KCC_USE_DIRECT_RECONCILERS=LoggingLogMetric \
GOLDEN_REQUEST_CHECKS=1 E2E_KUBE_TARGET=envtest E2E_GCP_TARGET=mock RUN_E2E=1 \
go test -test.count=1 -timeout 360s -v ./tests/e2e -run TestE2EScript/scenarios/fields

0 comments on commit 9a749a4

Please sign in to comment.