From 67ed1e336b7b185aba03992fd1c4fbebcd33941d Mon Sep 17 00:00:00 2001 From: Zorian Motso Date: Tue, 26 Sep 2023 14:12:01 +0300 Subject: [PATCH] feat!: Add valid error in status field of GitServer (#15) Removed all unnecessary properties from status as they were not used. Added "error" property to the status for storing errors if the connection wasn't established. BREAKING CHANGE: removed required fields from the GitServer status. CRD should be updated. Change-Id: If12029a5e9940f6f439e2face5d4905b6eef2e09 --- api/v1/git_server_types.go | 32 +---- api/v1/zz_generated.deepcopy.go | 3 +- .../crd/bases/v2.edp.epam.com_gitservers.yaml | 44 +------ config/rbac/role.yaml | 8 ++ controllers/gitserver/gitserver_controller.go | 123 ++++++++---------- .../gitserver/gitserver_controller_test.go | 92 ++++++------- controllers/gitserver/ssh.go | 39 ++---- .../crds/v2.edp.epam.com_gitservers.yaml | 44 +------ docs/api.md | 54 +------- main.go | 3 +- pkg/model/git-server.go | 19 --- pkg/model/git-server_test.go | 6 +- 12 files changed, 145 insertions(+), 322 deletions(-) diff --git a/api/v1/git_server_types.go b/api/v1/git_server_types.go index 93abd619..c8fc0c6f 100644 --- a/api/v1/git_server_types.go +++ b/api/v1/git_server_types.go @@ -38,40 +38,20 @@ type GitServerSpec struct { // GitServerStatus defines the observed state of GitServer. type GitServerStatus struct { - // This flag indicates neither JiraServer are initialized and ready to work. Defaults to false. - Available bool `json:"available"` - - // Information when the last time the action were performed. - LastTimeUpdated metaV1.Time `json:"last_time_updated"` - - // Specifies a current status of GitServer. - Status string `json:"status"` - - // Name of user who made a last change. - Username string `json:"username"` - - // The last Action was performed. - Action string `json:"action"` - - // A result of an action which were performed. - // - "success": action where performed successfully; - // - "error": error has occurred; - Result string `json:"result"` - - // Detailed information regarding action result - // which were performed + // Error represents error message if something went wrong. // +optional - DetailedMessage string `json:"detailed_message,omitempty"` + Error string `json:"error,omitempty"` - // Specifies a current state of GitServer. - Value string `json:"value"` + // Connected shows if operator is connected to git server. + // +optional + Connected bool `json:"connected"` } // +kubebuilder:object:root=true // +kubebuilder:subresource:status // +kubebuilder:resource:shortName=gs // +kubebuilder:storageversion -// +kubebuilder:printcolumn:name="Available",type="boolean",JSONPath=".status.available",description="Is resource available" +// +kubebuilder:printcolumn:name="Connected",type="boolean",JSONPath=".status.connected",description="Is connected to git server" // +kubebuilder:printcolumn:name="Host",type="string",JSONPath=".spec.gitHost",description="GitSever host" // +kubebuilder:printcolumn:name="Git Provider",type="string",JSONPath=".spec.gitProvider",description="Git Provider type" diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index 15503a4e..fc0da0e5 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -472,7 +472,7 @@ func (in *GitServer) DeepCopyInto(out *GitServer) { out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) out.Spec = in.Spec - in.Status.DeepCopyInto(&out.Status) + out.Status = in.Status } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GitServer. @@ -543,7 +543,6 @@ func (in *GitServerSpec) DeepCopy() *GitServerSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *GitServerStatus) DeepCopyInto(out *GitServerStatus) { *out = *in - in.LastTimeUpdated.DeepCopyInto(&out.LastTimeUpdated) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GitServerStatus. diff --git a/config/crd/bases/v2.edp.epam.com_gitservers.yaml b/config/crd/bases/v2.edp.epam.com_gitservers.yaml index 4964fa46..6bb929b8 100644 --- a/config/crd/bases/v2.edp.epam.com_gitservers.yaml +++ b/config/crd/bases/v2.edp.epam.com_gitservers.yaml @@ -18,9 +18,9 @@ spec: scope: Namespaced versions: - additionalPrinterColumns: - - description: Is resource available - jsonPath: .status.available - name: Available + - description: Is connected to git server + jsonPath: .status.connected + name: Connected type: boolean - description: GitSever host jsonPath: .spec.gitHost @@ -86,42 +86,12 @@ spec: status: description: GitServerStatus defines the observed state of GitServer. properties: - action: - description: The last Action was performed. - type: string - available: - description: This flag indicates neither JiraServer are initialized - and ready to work. Defaults to false. + connected: + description: Connected shows if operator is connected to git server. type: boolean - detailed_message: - description: Detailed information regarding action result which were - performed - type: string - last_time_updated: - description: Information when the last time the action were performed. - format: date-time - type: string - result: - description: 'A result of an action which were performed. - "success": - action where performed successfully; - "error": error has occurred;' + error: + description: Error represents error message if something went wrong. type: string - status: - description: Specifies a current status of GitServer. - type: string - username: - description: Name of user who made a last change. - type: string - value: - description: Specifies a current state of GitServer. - type: string - required: - - action - - available - - last_time_updated - - result - - status - - username - - value type: object type: object served: true diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index e3435028..0d1214e1 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -65,6 +65,14 @@ metadata: name: manager-role namespace: placeholder rules: +- apiGroups: + - "" + resources: + - secrets + verbs: + - get + - list + - watch - apiGroups: - argoproj.io resources: diff --git a/controllers/gitserver/gitserver_controller.go b/controllers/gitserver/gitserver_controller.go index 5e417696..1ad5f76f 100644 --- a/controllers/gitserver/gitserver_controller.go +++ b/controllers/gitserver/gitserver_controller.go @@ -5,14 +5,12 @@ import ( "fmt" "time" - "github.com/go-logr/logr" + coreV1 "k8s.io/api/core/v1" k8sErrors "k8s.io/apimachinery/pkg/api/errors" - metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/event" - "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" codebaseApi "github.com/epam/edp-codebase-operator/v2/api/v1" @@ -20,43 +18,26 @@ import ( codebasepredicate "github.com/epam/edp-codebase-operator/v2/pkg/predicate" ) -func NewReconcileGitServer(c client.Client, log logr.Logger) *ReconcileGitServer { +const ( + defaultRequeueTime = time.Second * 30 + successRequeueTime = time.Minute * 30 +) + +func NewReconcileGitServer(c client.Client) *ReconcileGitServer { return &ReconcileGitServer{ client: c, - log: log.WithName("git-server"), } } type ReconcileGitServer struct { client client.Client - log logr.Logger } func (r *ReconcileGitServer) SetupWithManager(mgr ctrl.Manager) error { - p := predicate.Funcs{ - UpdateFunc: func(e event.UpdateEvent) bool { - oldObject, ok := e.ObjectOld.(*codebaseApi.GitServer) - if !ok { - return false - } - - newObject, ok := e.ObjectNew.(*codebaseApi.GitServer) - if !ok { - return false - } - - if codebasepredicate.PauseAnnotationChanged(oldObject, newObject) { - return true - } - - return oldObject.Status == newObject.Status - }, - } - - pause := codebasepredicate.NewPause(r.log) + pause := codebasepredicate.NewPause(ctrl.Log.WithName("git-server-pause-predicate")) err := ctrl.NewControllerManagedBy(mgr). - For(&codebaseApi.GitServer{}, builder.WithPredicates(pause, p)). + For(&codebaseApi.GitServer{}, builder.WithPredicates(pause)). Complete(r) if err != nil { return fmt.Errorf("failed to build GitServer controller: %w", err) @@ -68,6 +49,7 @@ func (r *ReconcileGitServer) SetupWithManager(mgr ctrl.Manager) error { //+kubebuilder:rbac:groups=v2.edp.epam.com,namespace=placeholder,resources=gitservers,verbs=get;list;watch;create;update;patch;delete //+kubebuilder:rbac:groups=v2.edp.epam.com,namespace=placeholder,resources=gitservers/status,verbs=get;update;patch //+kubebuilder:rbac:groups=v2.edp.epam.com,namespace=placeholder,resources=gitservers/finalizers,verbs=update +//+kubebuilder:rbac:groups="",namespace=placeholder,resources=secrets,verbs=get;list;watch // Reconcile reads that state of the cluster for a GitServer object and makes changes based on the state. func (r *ReconcileGitServer) Reconcile(ctx context.Context, request reconcile.Request) (reconcile.Result, error) { @@ -75,80 +57,79 @@ func (r *ReconcileGitServer) Reconcile(ctx context.Context, request reconcile.Re log.Info("Reconciling GitServer") instance := &codebaseApi.GitServer{} - - err := r.client.Get(ctx, request.NamespacedName, instance) - if err != nil { + if err := r.client.Get(ctx, request.NamespacedName, instance); err != nil { if k8sErrors.IsNotFound(err) { - // Request object not found, could have been deleted after reconcile request. - // Owned objects are automatically garbage collected. For additional cleanup logic use finalizers. - // Return and don't requeue return reconcile.Result{}, nil } return reconcile.Result{}, fmt.Errorf("failed to fetch resource %q: %w", request.NamespacedName, err) } + oldStatus := instance.Status gitServer := model.ConvertToGitServer(instance) - hasConnection, err := checkConnectionToGitServer(r.client, gitServer, log) - if err != nil { - if updateErr := r.updateStatus(ctx, r.client, instance, hasConnection); updateErr != nil { - log.Error(updateErr, "failed to update GitServer status") + if err := r.checkConnectionToGitServer(ctx, gitServer); err != nil { + instance.Status.Error = err.Error() + instance.Status.Connected = false + + if statusErr := r.updateGitServerStatus(ctx, instance, oldStatus); statusErr != nil { + return reconcile.Result{}, statusErr } - return reconcile.Result{}, fmt.Errorf("failed to check connection to Git Server %v: %w", gitServer.GitHost, err) - } + log.Error(err, "GitServer connection is not established") - if err := r.updateStatus(ctx, r.client, instance, hasConnection); err != nil { - return reconcile.Result{}, fmt.Errorf("failed to update GitServer status %v: %w", gitServer.GitHost, err) + return reconcile.Result{RequeueAfter: defaultRequeueTime}, nil } - if !hasConnection { - const requeueTime = 30 * time.Second + instance.Status.Error = "" + instance.Status.Connected = true - log.Info("GitServer does not have connection, will try again later") - - return reconcile.Result{RequeueAfter: requeueTime}, nil + if err := r.updateGitServerStatus(ctx, instance, oldStatus); err != nil { + return reconcile.Result{}, err } log.Info("Reconciling GitServer has been finished") - return reconcile.Result{}, nil + return reconcile.Result{ + RequeueAfter: successRequeueTime, + }, nil } -func (*ReconcileGitServer) updateStatus(ctx context.Context, c client.Client, instance *codebaseApi.GitServer, hasConnection bool) error { +func (r *ReconcileGitServer) checkConnectionToGitServer(ctx context.Context, gitServer *model.GitServer) error { log := ctrl.LoggerFrom(ctx) + log.Info("Start CheckConnectionToGitServer method", "host", gitServer.GitHost) - instance.Status = generateStatus(hasConnection) + sshSecret := &coreV1.Secret{} - err := c.Status().Update(ctx, instance) + err := r.client.Get(ctx, types.NamespacedName{ + Namespace: gitServer.Namespace, + Name: gitServer.NameSshKeySecret, + }, sshSecret) if err != nil { - _ = c.Update(ctx, instance) + return fmt.Errorf("failed to get secret %s: %w", gitServer.NameSshKeySecret, err) + } + + sshData := extractSshData(gitServer, sshSecret) + + log.Info("Data from request is extracted", "host", sshData.Host, "port", sshData.Port) + + if err = checkGitServerConnection(sshData, log); err != nil { + return fmt.Errorf("failed to establish connection to Git Server %s: %w", sshData.Host, err) } - log.Info("Status for GitServer is set up.") + log.Info("Git server connection is established", "host", sshData.Host) return nil } -func generateStatus(hasConnection bool) codebaseApi.GitServerStatus { - if hasConnection { - return codebaseApi.GitServerStatus{ - Status: "created", - Available: hasConnection, - LastTimeUpdated: metaV1.Now(), - Result: "success", - Username: "system", - Value: "active", - } +func (r *ReconcileGitServer) updateGitServerStatus(ctx context.Context, gitServer *codebaseApi.GitServer, oldStatus codebaseApi.GitServerStatus) error { + if gitServer.Status == oldStatus { + return nil } - return codebaseApi.GitServerStatus{ - Status: "created", - Available: hasConnection, - LastTimeUpdated: metaV1.Now(), - Result: "error", - Username: "system", - Value: "inactive", + if err := r.client.Status().Update(ctx, gitServer); err != nil { + return fmt.Errorf("failed to update GitServer status: %w", err) } + + return nil } diff --git a/controllers/gitserver/gitserver_controller_test.go b/controllers/gitserver/gitserver_controller_test.go index f435e946..93ddeda8 100644 --- a/controllers/gitserver/gitserver_controller_test.go +++ b/controllers/gitserver/gitserver_controller_test.go @@ -4,12 +4,11 @@ import ( "context" "strings" "testing" - "time" "github.com/go-logr/logr" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - coreV1 "k8s.io/api/core/v1" + corev1 "k8s.io/api/core/v1" metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -30,7 +29,6 @@ func TestReconcileGitServer_Reconcile_ShouldPassNotFound(t *testing.T) { scheme.AddKnownTypes(codebaseApi.GroupVersion, gs) fakeCl := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(gs).Build() - // request req := reconcile.Request{ NamespacedName: types.NamespacedName{ Name: "NewGitServer", @@ -40,10 +38,9 @@ func TestReconcileGitServer_Reconcile_ShouldPassNotFound(t *testing.T) { r := ReconcileGitServer{ client: fakeCl, - log: logr.Discard(), } - res, err := r.Reconcile(context.TODO(), req) + res, err := r.Reconcile(ctrl.LoggerInto(context.Background(), logr.Discard()), req) assert.NoError(t, err) assert.False(t, res.Requeue) @@ -53,7 +50,6 @@ func TestReconcileGitServer_Reconcile_ShouldFailNotFound(t *testing.T) { scheme := runtime.NewScheme() fakeCl := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects().Build() - // request req := reconcile.Request{ NamespacedName: types.NamespacedName{ Name: "NewGitServer", @@ -63,10 +59,9 @@ func TestReconcileGitServer_Reconcile_ShouldFailNotFound(t *testing.T) { r := ReconcileGitServer{ client: fakeCl, - log: logr.Discard(), } - res, err := r.Reconcile(context.TODO(), req) + res, err := r.Reconcile(ctrl.LoggerInto(context.Background(), logr.Discard()), req) assert.Error(t, err) @@ -89,10 +84,11 @@ func TestReconcileGitServer_Reconcile_ShouldFailToGetSecret(t *testing.T) { }, } scheme := runtime.NewScheme() - scheme.AddKnownTypes(codebaseApi.GroupVersion, gs) + require.NoError(t, corev1.AddToScheme(scheme)) + require.NoError(t, codebaseApi.AddToScheme(scheme)) + fakeCl := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(gs).Build() - // request req := reconcile.Request{ NamespacedName: types.NamespacedName{ Name: "NewGitServer", @@ -102,18 +98,17 @@ func TestReconcileGitServer_Reconcile_ShouldFailToGetSecret(t *testing.T) { r := ReconcileGitServer{ client: fakeCl, - log: logr.Discard(), } - res, err := r.Reconcile(context.TODO(), req) - - assert.Error(t, err) + logger := platform.NewLoggerMock() + loggerSink, ok := logger.GetSink().(*platform.LoggerMock) + require.True(t, ok) - if !strings.Contains(err.Error(), "failed to get ssh-secret secret") { - t.Fatalf("wrong error returned: %s", err.Error()) - } + _, err := r.Reconcile(ctrl.LoggerInto(context.Background(), logger), req) - assert.False(t, res.Requeue) + require.NoError(t, err) + require.Error(t, loggerSink.LastError()) + assert.Contains(t, loggerSink.LastError().Error(), "failed to get secret ssh-secret") } func TestReconcileGitServer_UpdateStatus_ShouldPassWithSuccess(t *testing.T) { @@ -133,16 +128,20 @@ func TestReconcileGitServer_UpdateStatus_ShouldPassWithSuccess(t *testing.T) { r := ReconcileGitServer{ client: fakeCl, - log: logr.Discard(), } - err := r.updateStatus(context.TODO(), fakeCl, gs, true) + err := r.updateGitServerStatus( + ctrl.LoggerInto(context.Background(), logr.Discard()), + gs, + codebaseApi.GitServerStatus{ + Connected: true, + }, + ) assert.NoError(t, err) - assert.Equal(t, gs.Status.Result, "success") } -func TestReconcileGitServer_UpdateStatus_ShouldPassWithFailure(t *testing.T) { +func TestReconcileGitServer_UpdateStatus_Failure(t *testing.T) { gs := &codebaseApi.GitServer{ ObjectMeta: metaV1.ObjectMeta{ Name: "NewGitServer", @@ -159,13 +158,17 @@ func TestReconcileGitServer_UpdateStatus_ShouldPassWithFailure(t *testing.T) { r := ReconcileGitServer{ client: fakeCl, - log: logr.Discard(), } - err := r.updateStatus(context.TODO(), fakeCl, gs, false) + err := r.updateGitServerStatus( + ctrl.LoggerInto(context.Background(), logr.Discard()), + &codebaseApi.GitServer{}, + codebaseApi.GitServerStatus{ + Connected: true, + }, + ) - assert.NoError(t, err) - assert.Equal(t, gs.Status.Result, "error") + assert.Error(t, err) } const testKey = `-----BEGIN OPENSSH PRIVATE KEY----- @@ -220,10 +223,10 @@ func TestReconcileGitServer_ServerUnavailable(t *testing.T) { } scheme := runtime.NewScheme() scheme.AddKnownTypes(codebaseApi.GroupVersion, gs) - err := coreV1.AddToScheme(scheme) + err := corev1.AddToScheme(scheme) assert.NoError(t, err) - secret := coreV1.Secret{ObjectMeta: metaV1.ObjectMeta{Name: "ssh-secret", Namespace: gs.Namespace}, Data: map[string][]byte{ + secret := corev1.Secret{ObjectMeta: metaV1.ObjectMeta{Name: "ssh-secret", Namespace: gs.Namespace}, Data: map[string][]byte{ util.PrivateSShKeyName: []byte(testKey), }} @@ -235,7 +238,6 @@ func TestReconcileGitServer_ServerUnavailable(t *testing.T) { r := ReconcileGitServer{ client: fakeCl, - log: logger, } req := reconcile.Request{ @@ -247,15 +249,14 @@ func TestReconcileGitServer_ServerUnavailable(t *testing.T) { res, err := r.Reconcile(ctrl.LoggerInto(context.Background(), logger), req) assert.NoError(t, err) - assert.Equal(t, res.RequeueAfter, time.Second*30) - - _, ok = loggerSink.InfoMessages()["GitServer does not have connection, will try again later"] - assert.True(t, ok) + assert.Equal(t, res.RequeueAfter, defaultRequeueTime) + require.Error(t, loggerSink.LastError()) + assert.Contains(t, loggerSink.LastError().Error(), "failed to establish connection to Git Server") } func TestReconcileGitServer_InvalidSSHKey(t *testing.T) { scheme := runtime.NewScheme() - err := coreV1.AddToScheme(scheme) + err := corev1.AddToScheme(scheme) require.NoError(t, err) err = codebaseApi.AddToScheme(scheme) @@ -272,7 +273,7 @@ func TestReconcileGitServer_InvalidSSHKey(t *testing.T) { }, } - secret := &coreV1.Secret{ + secret := &corev1.Secret{ ObjectMeta: metaV1.ObjectMeta{ Name: "ssh-secret", Namespace: gs.Namespace, @@ -284,11 +285,8 @@ func TestReconcileGitServer_InvalidSSHKey(t *testing.T) { fakeCl := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(gs, secret).Build() - logger := platform.NewLoggerMock() - r := ReconcileGitServer{ client: fakeCl, - log: logger, } req := reconcile.Request{ @@ -298,14 +296,19 @@ func TestReconcileGitServer_InvalidSSHKey(t *testing.T) { }, } - _, err = r.Reconcile(context.Background(), req) - require.Error(t, err) - require.Contains(t, err.Error(), "failed to parse private key") + logger := platform.NewLoggerMock() + loggerSink, ok := logger.GetSink().(*platform.LoggerMock) + require.True(t, ok) + + _, err = r.Reconcile(ctrl.LoggerInto(context.Background(), logger), req) + require.NoError(t, err) + require.Error(t, loggerSink.LastError()) + require.Contains(t, loggerSink.LastError().Error(), "failed to parse private key") gotGitServer := &codebaseApi.GitServer{} err = fakeCl.Get(context.Background(), req.NamespacedName, gotGitServer) require.NoError(t, err) - assert.False(t, gotGitServer.Status.Available) + assert.False(t, gotGitServer.Status.Connected) } func TestNewReconcileGitServer(t *testing.T) { @@ -335,14 +338,11 @@ func TestNewReconcileGitServer(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - log := logr.Discard() - want := &ReconcileGitServer{ client: tt.args.c, - log: log, } - got := NewReconcileGitServer(tt.args.c, log) + got := NewReconcileGitServer(tt.args.c) assert.Equal(t, want, got) }) diff --git a/controllers/gitserver/ssh.go b/controllers/gitserver/ssh.go index e69aaf05..f4bb970f 100644 --- a/controllers/gitserver/ssh.go +++ b/controllers/gitserver/ssh.go @@ -6,7 +6,6 @@ import ( "github.com/go-logr/logr" "golang.org/x/crypto/ssh" corev1 "k8s.io/api/core/v1" - "sigs.k8s.io/controller-runtime/pkg/client" "github.com/epam/edp-codebase-operator/v2/pkg/gerrit" "github.com/epam/edp-codebase-operator/v2/pkg/model" @@ -20,35 +19,13 @@ type gitSshData struct { Port int32 } -func checkConnectionToGitServer(c client.Client, gitServer *model.GitServer, log logr.Logger) (bool, error) { - log.Info("Start CheckConnectionToGitServer method", "Git host", gitServer.GitHost) - - sshSecret, err := util.GetSecret(c, gitServer.NameSshKeySecret, gitServer.Namespace) - if err != nil { - return false, fmt.Errorf("failed to get %v secret: %w", gitServer.NameSshKeySecret, err) - } - - sshData := extractSshData(gitServer, sshSecret) - - log.Info("Data from request is extracted", "host", sshData.Host, "port", sshData.Port) - - accessible, err := isGitServerAccessible(sshData, log) - if err != nil { - return false, fmt.Errorf("an error has occurred while checking connection to git server: %w", err) - } - - log.Info("Git server", "accessible", accessible) - - return accessible, nil -} - -func isGitServerAccessible(data gitSshData, log logr.Logger) (bool, error) { +// checkGitServerConnection checks connection to Git server. If connection is not established, returns error. +func checkGitServerConnection(data gitSshData, log logr.Logger) error { log.Info("Start executing IsGitServerAccessible method to check connection to server", "host", data.Host) sshClient, err := sshInitFromSecret(data, log) if err != nil { - log.Info(fmt.Sprintf("An error has occurred while initing SSH client. Check data in Git Server resource and secret: %v", err)) - return false, err + return fmt.Errorf("failed to initialize ssh client: %w", err) } var ( @@ -57,14 +34,16 @@ func isGitServerAccessible(data gitSshData, log logr.Logger) (bool, error) { ) if s, c, err = sshClient.NewSession(); err != nil { - log.Info(fmt.Sprintf("An error has occurred while connecting to server. Check data in Git Server resource and secret: %v", err)) - return false, nil + return fmt.Errorf("failed to create ssh session: %w", err) } - defer util.CloseWithLogOnErr(log, s, "failed to close ssh client session") defer util.CloseWithLogOnErr(log, c, "failed to close ssh client connection") - return s != nil && c != nil, nil + if s != nil { + defer util.CloseWithLogOnErr(log, s, "failed to close ssh client session") + } + + return nil } func extractSshData(gitServer *model.GitServer, secret *corev1.Secret) gitSshData { diff --git a/deploy-templates/crds/v2.edp.epam.com_gitservers.yaml b/deploy-templates/crds/v2.edp.epam.com_gitservers.yaml index 4964fa46..6bb929b8 100644 --- a/deploy-templates/crds/v2.edp.epam.com_gitservers.yaml +++ b/deploy-templates/crds/v2.edp.epam.com_gitservers.yaml @@ -18,9 +18,9 @@ spec: scope: Namespaced versions: - additionalPrinterColumns: - - description: Is resource available - jsonPath: .status.available - name: Available + - description: Is connected to git server + jsonPath: .status.connected + name: Connected type: boolean - description: GitSever host jsonPath: .spec.gitHost @@ -86,42 +86,12 @@ spec: status: description: GitServerStatus defines the observed state of GitServer. properties: - action: - description: The last Action was performed. - type: string - available: - description: This flag indicates neither JiraServer are initialized - and ready to work. Defaults to false. + connected: + description: Connected shows if operator is connected to git server. type: boolean - detailed_message: - description: Detailed information regarding action result which were - performed - type: string - last_time_updated: - description: Information when the last time the action were performed. - format: date-time - type: string - result: - description: 'A result of an action which were performed. - "success": - action where performed successfully; - "error": error has occurred;' + error: + description: Error represents error message if something went wrong. type: string - status: - description: Specifies a current status of GitServer. - type: string - username: - description: Name of user who made a last change. - type: string - value: - description: Specifies a current state of GitServer. - type: string - required: - - action - - available - - last_time_updated - - result - - status - - username - - value type: object type: object served: true diff --git a/docs/api.md b/docs/api.md index d22f5289..f3719b1c 100644 --- a/docs/api.md +++ b/docs/api.md @@ -1185,61 +1185,17 @@ GitServerStatus defines the observed state of GitServer. - action - string - - The last Action was performed.
- - true - - available + connected boolean - This flag indicates neither JiraServer are initialized and ready to work. Defaults to false.
- - true - - last_time_updated - string - - Information when the last time the action were performed.
-
- Format: date-time
- - true - - result - string - - A result of an action which were performed. - "success": action where performed successfully; - "error": error has occurred;
- - true - - status - string - - Specifies a current status of GitServer.
+ Connected shows if operator is connected to git server.
- true - - username - string - - Name of user who made a last change.
- - true - - value - string - - Specifies a current state of GitServer.
- - true + false - detailed_message + error string - Detailed information regarding action result which were performed
+ Error represents error message if something went wrong.
false diff --git a/main.go b/main.go index 527c48f2..0eec48b0 100644 --- a/main.go +++ b/main.go @@ -154,8 +154,7 @@ func main() { os.Exit(1) } - gitServerCtrl := gitserver.NewReconcileGitServer(mgr.GetClient(), ctrlLog) - if err = gitServerCtrl.SetupWithManager(mgr); err != nil { + if err = gitserver.NewReconcileGitServer(mgr.GetClient()).SetupWithManager(mgr); err != nil { setupLog.Error(err, logFailCtrlCreateMessage, "controller", "git-server") os.Exit(1) } diff --git a/pkg/model/git-server.go b/pkg/model/git-server.go index 388517b9..273ff102 100644 --- a/pkg/model/git-server.go +++ b/pkg/model/git-server.go @@ -1,7 +1,6 @@ package model import ( - "strings" "time" ctrl "sigs.k8s.io/controller-runtime" @@ -46,31 +45,13 @@ func ConvertToGitServer(k8sObj *codebaseApi.GitServer) *GitServer { spec := k8sObj.Spec - actionLog := convertGitServerActionLog(&k8sObj.Status) - return &GitServer{ GitHost: spec.GitHost, GitUser: spec.GitUser, HttpsPort: spec.HttpsPort, SshPort: spec.SshPort, NameSshKeySecret: spec.NameSshKeySecret, - ActionLog: *actionLog, Namespace: k8sObj.Namespace, Name: k8sObj.Name, } } - -func convertGitServerActionLog(status *codebaseApi.GitServerStatus) *ActionLog { - return &ActionLog{ - Event: formatStatus(status.Status), - DetailedMessage: status.DetailedMessage, - Username: status.Username, - UpdatedAt: status.LastTimeUpdated.Time, - Action: status.Action, - Result: status.Result, - } -} - -func formatStatus(status string) string { - return strings.ToLower(strings.ReplaceAll(status, " ", "_")) -} diff --git a/pkg/model/git-server_test.go b/pkg/model/git-server_test.go index 76b401c2..ca71dcdf 100644 --- a/pkg/model/git-server_test.go +++ b/pkg/model/git-server_test.go @@ -12,10 +12,10 @@ func TestConvertToGitServer(t *testing.T) { t.Parallel() gs := ConvertToGitServer(&codebaseApi.GitServer{ - Status: codebaseApi.GitServerStatus{ - Status: "hello world", + Spec: codebaseApi.GitServerSpec{ + GitHost: "github.com", }, }) - assert.Equal(t, gs.ActionLog.Event, "hello_world") + assert.Equal(t, gs.GitHost, "github.com") }