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

Refactor watcher and kyma reconcilers to improve load test performance #277

Merged
merged 66 commits into from
Nov 11, 2022
Merged
Show file tree
Hide file tree
Changes from 64 commits
Commits
Show all changes
66 commits
Select commit Hold shift + click to select a range
61205a6
refactor deploy and custom pkgs for the new watcher reconciler impl
fourthisle Oct 18, 2022
8b190af
refactor watcher reconciler to reconcile kyma resources
fourthisle Oct 20, 2022
3397e6a
Merge remote-tracking branch 'upstream/main' into watcher-reconciles-…
fourthisle Oct 20, 2022
63f4f77
go mod tidy
fourthisle Oct 20, 2022
8f1dbaa
fix nil httpRoute bug
fourthisle Oct 20, 2022
f16c728
disable watcher cr state handling
fourthisle Oct 20, 2022
fe89be7
fix first bug in ACs
fourthisle Oct 21, 2022
24476ad
add pre-install check for skr webhook chart
fourthisle Oct 24, 2022
7223aee
add condition for skrwebhook installation
ruanxin Oct 24, 2022
1c5b9bf
Merge remote-tracking branch 'khlifi411/watcher-reconciles-kymas' int…
ruanxin Oct 24, 2022
dcc29ea
refactor skr chart config out from kyma reconciler
fourthisle Oct 24, 2022
94f66d9
Merge remote-tracking branch 'origin/watcher-reconciles-kymas' into w…
fourthisle Oct 24, 2022
9c88194
fix tests for deploy pkg
fourthisle Oct 24, 2022
6f8f208
fix failing tests for watcher ctrl
fourthisle Oct 24, 2022
935f6fa
create a dedicate test suite for watcher
ruanxin Oct 24, 2022
a168513
refactor watcher controller tests to run with kyma controller
fourthisle Oct 25, 2022
b61bf95
remove commented code
fourthisle Oct 25, 2022
6f3398b
move creating istio resources to suite setup for with_watcher tests
fourthisle Oct 25, 2022
00df998
remove duplicate tests and fix kyma reconciler _with_watcher tests
fourthisle Oct 25, 2022
5faf3ef
set MaxConcurrentReconciles to 1 for WatcherReconciler
fourthisle Oct 25, 2022
7c7fa74
reuse duplicate methods in kyma reconciler tests
fourthisle Oct 25, 2022
7cb2fbb
Merge remote-tracking branch 'upstream/main' into watcher-reconciles-…
fourthisle Oct 25, 2022
805e0ed
remove module catalog reconciler from watcher suite
fourthisle Oct 25, 2022
865d883
cleanup unused methods
fourthisle Oct 25, 2022
ab6376a
reuse test helper functions in kyma controller tests
fourthisle Oct 26, 2022
882016e
remove secret lookup for load test
fourthisle Oct 26, 2022
d164389
remove underscore from package names to fix lint issues
fourthisle Oct 26, 2022
8c1c346
fix linting issues
fourthisle Oct 26, 2022
69b7c0d
Merge remote-tracking branch 'upstream/main' into watcher-reconciles-…
fourthisle Oct 27, 2022
7a88afa
remove unnecessary imports
fourthisle Oct 27, 2022
273a19e
implement review comment
fourthisle Oct 27, 2022
1701e6c
use errors.New for non-wrapped errors
fourthisle Oct 27, 2022
55ee856
move indirect dep to a direct one
fourthisle Oct 27, 2022
0c90215
fix imports formatting
fourthisle Oct 27, 2022
b540793
Merge remote-tracking branch 'upstream/main' into watcher-reconciles-…
fourthisle Oct 27, 2022
0b21cd5
add label to skr chart to trigger pod restart
fourthisle Oct 28, 2022
faa2b62
add trigger label as a chart arg for the SKR webhook
fourthisle Oct 28, 2022
0cf3cd4
fix failing test
fourthisle Oct 28, 2022
b0d225a
fix label value
fourthisle Oct 28, 2022
2b80146
implement review suggestions
fourthisle Nov 2, 2022
b41fd17
add chart manager config struct
fourthisle Nov 2, 2022
5c4b017
add go:build tag for testhelper package to exclude it from build
fourthisle Nov 3, 2022
ab0ac43
remove setIstioClient method from watcherReconciler
fourthisle Nov 3, 2022
af2e11a
reuse sync context when removing skr chart
fourthisle Nov 3, 2022
616aaeb
move SKRWebhookConfig to deploy package
fourthisle Nov 3, 2022
cfddc09
enhance comment explaining using one worker to reconcile watchers
fourthisle Nov 3, 2022
da3931d
Merge remote-tracking branch 'upstream/main' into watcher-reconciles-…
fourthisle Nov 3, 2022
c44823f
fix merge conflict errors
fourthisle Nov 3, 2022
d99ef76
fix linting issues
fourthisle Nov 3, 2022
7428d7a
remove unused method
fourthisle Nov 3, 2022
acc4045
migrate testhelper to internal pkg
fourthisle Nov 3, 2022
ad33d6a
refactor enable watcher bool out of kyma reconciler
fourthisle Nov 3, 2022
b5640b1
fix linting errors
fourthisle Nov 3, 2022
7f86a7a
fix error condition bug
fourthisle Nov 3, 2022
325ce2c
migrate skr chart from out of internal folder
fourthisle Nov 7, 2022
8ea16ee
enable watcher by default in the watcher overlay
fourthisle Nov 7, 2022
df95423
add flags for vsname and gateway name
fourthisle Nov 7, 2022
1c12be3
remove webhook-pre-install-check flag
fourthisle Nov 7, 2022
489896c
Merge remote-tracking branch 'upstream/main' into watcher-reconciles-…
fourthisle Nov 9, 2022
2606b1d
implement missing watcher test and helm client cache
fourthisle Nov 9, 2022
96f81bc
fix linting issue
fourthisle Nov 9, 2022
d59bd59
not passing cache as param to library calls
fourthisle Nov 9, 2022
62c2d87
Merge remote-tracking branch 'upstream/main' into watcher-reconciles-…
fourthisle Nov 9, 2022
90752f0
fix linting
fourthisle Nov 9, 2022
29bf441
Merge remote-tracking branch 'upstream/main' into watcher-reconciles-…
fourthisle Nov 11, 2022
7547a26
skip failing test caused by manifest lib
fourthisle Nov 11, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@
atlassian-ide-plugin.xml

## VSCode
.vscode
**/*.vscode
**/*.devcontainer*

### OSX template
*.DS_Store
Expand Down Expand Up @@ -90,5 +91,8 @@ out.txt
**/go.work
**/go.work.sum

# ignore .pem key files
**/*.pem

# rendered manifest
**/manifest.yaml
2 changes: 1 addition & 1 deletion operator/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ RUN CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build -a -o manager main.go
FROM gcr.io/distroless/static:nonroot
WORKDIR /
COPY --from=builder /workspace/manager .
COPY --from=builder /workspace/internal/charts/skr-webhook ./skr-webhook
COPY charts/skr-webhook charts/skr-webhook
USER 65532:65532

ENTRYPOINT ["/manager"]
18 changes: 18 additions & 0 deletions operator/api/v1alpha1/condition_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,15 @@ const (
MessageModuleNotInReadyState = "not all modules are in ready state"
MessageModuleCatalogIsSynced = "module catalog is synchronized"
MessageModuleCatalogIsOutOfSync = "module catalog is out of sync and needs to be resynchronized"
MessageSKRWebhookIsSynced = "skrwebhook is synchronized"
MessageSKRWebhookIsOutOfSync = "skrwebhook is out of sync and needs to be resynchronized"
)

// Extend this list by actual needs.
const (
ConditionReasonModulesAreReady KymaConditionReason = "ModulesAreReady"
ConditionReasonModuleCatalogIsReady KymaConditionReason = "ModuleCatalogIsReady"
ConditionReasonSKRWebhookIsReady KymaConditionReason = "SKRWebhookIsReady"
)

func NewConditionBuilder() *ConditionBuilder {
Expand Down Expand Up @@ -69,6 +78,15 @@ func (cb *ConditionBuilder) generateMessage() string {
}

return MessageModuleCatalogIsOutOfSync
case ConditionReasonSKRWebhookIsReady:
switch cb.Status {
case metav1.ConditionTrue:
return MessageSKRWebhookIsSynced
case metav1.ConditionUnknown:
case metav1.ConditionFalse:
}

return MessageSKRWebhookIsOutOfSync
}

return "no detailed message available as reason is unknown to API"
Expand Down
6 changes: 0 additions & 6 deletions operator/api/v1alpha1/kyma_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -249,12 +249,6 @@ const (
// and the actual state of individual module can be found in related ModuleStatus.
type KymaConditionReason string

// Extend this list by actual needs.
const (
ConditionReasonModulesAreReady KymaConditionReason = "ModulesAreReady"
ConditionReasonModuleCatalogIsReady KymaConditionReason = "ModuleCatalogIsReady"
)

//+genclient
//+kubebuilder:object:root=true
//+kubebuilder:subresource:status
Expand Down
31 changes: 0 additions & 31 deletions operator/api/v1alpha1/watcher_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ limitations under the License.
package v1alpha1

import (
"time"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

Expand Down Expand Up @@ -143,8 +141,6 @@ const (
)

//+kubebuilder:object:root=true
//+kubebuilder:subresource:status
//+kubebuilder:printcolumn:name="State",type=string,JSONPath=".status.state"
//+kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp"

// Watcher is the Schema for the watchers API.
Expand All @@ -156,14 +152,6 @@ type Watcher struct {

// +kubebuilder:validation:Optional
Spec WatcherSpec `json:"spec"`

// +kubebuilder:validation:Optional
Status WatcherStatus `json:"status"`
}

func (w *Watcher) SetObservedGeneration() *Watcher {
w.Status.ObservedGeneration = w.Generation
return w
}

func (w *Watcher) GetModuleName() string {
Expand All @@ -173,25 +161,6 @@ func (w *Watcher) GetModuleName() string {
return w.Labels[ManagedBylabel]
}

func (w *Watcher) AddOrUpdateReadyCondition(state WatcherConditionStatus, msg string) {
lastTransitionTime := &metav1.Time{Time: time.Now()}
if len(w.Status.Conditions) == 0 {
w.Status.Conditions = []WatcherCondition{{
Type: WatcherConditionTypeReady,
Status: state,
Message: msg,
LastTransitionTime: lastTransitionTime,
}}
}
for i := range w.Status.Conditions {
condition := &w.Status.Conditions[i]
if condition.Type == WatcherConditionTypeReady {
condition.Status = state
condition.LastTransitionTime = lastTransitionTime
}
}
}

//+kubebuilder:object:root=true

// WatcherList contains a list of Watcher.
Expand Down
1 change: 0 additions & 1 deletion operator/api/v1alpha1/zz_generated.deepcopy.go

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

Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ spec:
metadata:
labels:
app: skr-webhook
operator.kyma-project.io/pod-restart-trigger: "{{.Values.triggerLabel}}"
spec:
serviceAccountName: {{.Release.Name}}-webhook-sa
containers:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,13 @@ type: Opaque
data:
{{- $caCert := "" }}
{{- if .Values.tls.helmCertGen }}
{{- $data := (lookup "v1" "Secret" .Release.Namespace (printf "%s-webhook-tls" .Release.Name )).data }}
{{- if $data }}
{{ $data | toYaml | nindent 2 }}
{{- $caCert = index $data "ca.crt" }}
{{- else }}
{{- $cn := printf "%s-webhook.%s.svc" .Release.Name .Release.Namespace }}
{{- $ca := genCA (printf "%s-webhook-ca" .Release.Name ) 36500 }}
{{- $cert := genSignedCert $cn (list "127.0.0.1") (list $cn "localhost") 36500 $ca }}
ca.crt: {{ $ca.Cert | b64enc }}
tls.crt: {{ $cert.Cert | b64enc }}
tls.key: {{ $cert.Key | b64enc }}
{{- $caCert = $ca.Cert | b64enc }}
{{- end }}
{{- else }}
ca.crt: {{.Values.tls.caCert}}
tls.crt: {{.Values.tls.clientCert }}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
sidecar: true
# -- Replicas
replicas: 1
# -- Trigger pod restart
triggerLabel: ""
image:
# -- Image repository
repository: "eu.gcr.io/kyma-project"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,6 @@ spec:
scope: Namespaced
versions:
- additionalPrinterColumns:
- jsonPath: .status.state
name: State
type: string
- jsonPath: .metadata.creationTimestamp
name: Age
type: date
Expand Down Expand Up @@ -78,67 +75,7 @@ spec:
- labelsToWatch
- serviceInfo
type: object
status:
description: WatcherStatus defines the observed state of Watcher.
properties:
conditions:
description: List of status conditions to indicate the status of a
Watcher.
items:
description: WatcherCondition describes condition information for
Watcher.
properties:
lastTransitionTime:
description: Timestamp for when Watcher last transitioned from
one status to another.
format: date-time
type: string
message:
description: Human-readable message indicating details about
the last status transition.
type: string
reason:
description: Machine-readable text indicating the reason for
the condition's last transition.
type: string
status:
description: Status of the Watcher Condition. Value can be one
of ("True", "False", "Unknown").
enum:
- "True"
- "False"
- Unknown
type: string
type:
description: Type is used to reflect what type of condition
we are dealing with. Most commonly WatcherConditionTypeReady
it is used as extension marker in the future.
enum:
- Ready
type: string
required:
- status
- type
type: object
type: array
observedGeneration:
description: ObservedGeneration
format: int64
type: integer
state:
description: State signifies current state of a Watcher. Value can
be one of ("Ready", "Processing", "Error", "Deleting")
enum:
- Processing
- Deleting
- Ready
- Error
type: string
required:
- state
type: object
type: object
served: true
storage: true
subresources:
status: {}
subresources: {}
2 changes: 1 addition & 1 deletion operator/config/watcher/kustomization.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ patches:
- patch: |-
- op: add
path: /spec/template/spec/containers/0/args/-
value: --enable-kcp-watcher=false
value: --enable-kcp-watcher
- op: add
path: /spec/template/spec/containers/0/args/-
value: --skr-watcher-path=/skr-webhook
Expand Down
31 changes: 25 additions & 6 deletions operator/controllers/kyma_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,16 @@ package controllers

import (
"context"
"errors"
"fmt"
"time"

"github.com/kyma-project/lifecycle-manager/operator/internal/deploy"
"k8s.io/client-go/rest"

manifestV1alpha1 "github.com/kyma-project/module-manager/operator/api/v1alpha1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"github.com/kyma-project/lifecycle-manager/operator/api/v1alpha1"
"github.com/kyma-project/lifecycle-manager/operator/pkg/adapter"
"github.com/kyma-project/lifecycle-manager/operator/pkg/catalog"
Expand All @@ -31,8 +38,6 @@ import (
"github.com/kyma-project/lifecycle-manager/operator/pkg/remote"
"github.com/kyma-project/lifecycle-manager/operator/pkg/signature"
"github.com/kyma-project/lifecycle-manager/operator/pkg/status"
manifestV1alpha1 "github.com/kyma-project/module-manager/operator/api/v1alpha1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"k8s.io/client-go/tools/record"
ctrl "sigs.k8s.io/controller-runtime"
Expand All @@ -45,6 +50,8 @@ type EventErrorType string

const ModuleReconciliationError EventErrorType = "ModuleReconciliationError"

var ErrSkrChartConfigNotSet = errors.New("skr chart config is not set")

type RequeueIntervals struct {
Success time.Duration
}
Expand All @@ -56,6 +63,8 @@ type KymaReconciler struct {
RequeueIntervals
signature.VerificationSettings
RemoteClientCache *remote.ClientCache
deploy.SKRWebhookChartManager
KcpRestConfig *rest.Config
}

//nolint:lll
Expand Down Expand Up @@ -248,7 +257,14 @@ func (r *KymaReconciler) HandleProcessingState(ctx context.Context, kyma *v1alph
return r.UpdateStatusWithEventFromErr(ctx, kyma, v1alpha1.StateError,
fmt.Errorf("error while syncing conditions during deleting non exists modules: %w", err))
}

statusUpdateRequiredFromSKRWebhookSync := false
if kyma.Spec.Sync.Enabled {
if statusUpdateRequiredFromSKRWebhookSync, err = r.InstallWebhookChart(ctx, kyma,
r.RemoteClientCache, r.Client); err != nil {
kyma.UpdateCondition(v1alpha1.ConditionReasonSKRWebhookIsReady, metav1.ConditionFalse)
return err
}
}
kyma.SyncConditionsWithModuleStates()
// set ready condition if applicable
if kyma.AreAllConditionsReadyForKyma() && kyma.Status.State != v1alpha1.StateReady {
Expand All @@ -258,10 +274,10 @@ func (r *KymaReconciler) HandleProcessingState(ctx context.Context, kyma *v1alph
return r.UpdateStatusWithEvent(ctx, kyma, v1alpha1.StateReady, message)
}

isStatusUpdateRequired := statusUpdateRequiredFromModuleSync || statusUpdateRequiredFromModuleStatusSync ||
statusUpdateRequiredFromDeletion || statusUpdateRequiredFromSKRWebhookSync
// if the ready condition is not applicable, but we changed the conditions, we still need to issue an update
if statusUpdateRequiredFromModuleSync ||
statusUpdateRequiredFromModuleStatusSync ||
statusUpdateRequiredFromDeletion {
if isStatusUpdateRequired {
if err := r.UpdateStatusWithEvent(ctx, kyma, v1alpha1.StateProcessing, "updating component conditions"); err != nil {
return fmt.Errorf("error while updating status for condition change: %w", err)
}
Expand All @@ -279,6 +295,9 @@ func (r *KymaReconciler) HandleDeletingState(ctx context.Context, kyma *v1alpha1
if err != nil {
return false, fmt.Errorf("remote sync initialization failed: %w", err)
}
if err = r.RemoveWebhookChart(ctx, kyma, syncContext); err != nil {
return true, nil
}
force := true
if err := catalog.NewRemoteCatalog(
syncContext, catalog.Settings{
Expand Down
Loading