From 5fa786bcbf1e6f2fdc4d8be604f8880441f01d2b Mon Sep 17 00:00:00 2001 From: Tanmoy Sarkar <57363826+tanmoysrt@users.noreply.github.com> Date: Mon, 25 Mar 2024 03:19:41 +0530 Subject: [PATCH] fix: ineffassign fix (#429) --- .../process_ingress_rule_apply_request.go | 61 ++++--------------- .../process_ingress_rule_delete_request.go | 38 ++++-------- ..._install_dependencies_on_server_request.go | 12 +--- .../process_redirect_rule_apply_request.go | 28 +++------ .../process_redirect_rule_delete_request.go | 19 +++--- .../worker/process_ssl_request.go | 9 ++- 6 files changed, 50 insertions(+), 117 deletions(-) diff --git a/swiftwave_service/worker/process_ingress_rule_apply_request.go b/swiftwave_service/worker/process_ingress_rule_apply_request.go index 4e10411a9a..71307207a5 100644 --- a/swiftwave_service/worker/process_ingress_rule_apply_request.go +++ b/swiftwave_service/worker/process_ingress_rule_apply_request.go @@ -86,67 +86,43 @@ func (m Manager) IngressRuleApply(request IngressRuleApplyRequest, ctx context.C // add backend _, err = haproxyManager.AddBackend(haproxyTransactionId, application.Name, int(ingressRule.TargetPort), int(application.Replicas)) if err != nil { - //nolint:ineffassign isFailed = true - // set status as failed and exit - _ = ingressRule.UpdateStatus(ctx, dbWithoutTx, core.IngressRuleStatusFailed) - // no requeue - return nil + break } // add frontend if ingressRule.Protocol == core.HTTPSProtocol { err = haproxyManager.AddHTTPSLink(haproxyTransactionId, backendName, domain.Name) if err != nil { - //nolint:ineffassign isFailed = true - // set status as failed and exit - _ = ingressRule.UpdateStatus(ctx, dbWithoutTx, core.IngressRuleStatusFailed) - // no requeue - return nil + break } } else if ingressRule.Protocol == core.HTTPProtocol { // for default port 80, should use fe_http frontend due to some binding restrictions if ingressRule.Port == 80 { err = haproxyManager.AddHTTPLink(haproxyTransactionId, backendName, domain.Name) if err != nil { - //nolint:ineffassign isFailed = true - // set status as failed and exit - _ = ingressRule.UpdateStatus(ctx, dbWithoutTx, core.IngressRuleStatusFailed) - // no requeue - return nil + break } } else { // for other ports, use custom frontend err = haproxyManager.AddTCPLink(haproxyTransactionId, backendName, int(ingressRule.Port), domain.Name, haproxymanager.HTTPMode, restrictedPorts) if err != nil { - //nolint:ineffassign isFailed = true - // set status as failed and exit - _ = ingressRule.UpdateStatus(ctx, dbWithoutTx, core.IngressRuleStatusFailed) - // no requeue - return nil + break } } } else if ingressRule.Protocol == core.TCPProtocol { err = haproxyManager.AddTCPLink(haproxyTransactionId, backendName, int(ingressRule.Port), "", haproxymanager.TCPMode, restrictedPorts) if err != nil { - //nolint:ineffassign isFailed = true - // set status as failed and exit - _ = ingressRule.UpdateStatus(ctx, dbWithoutTx, core.IngressRuleStatusFailed) - // no requeue - return nil + break } } else if ingressRule.Protocol == core.UDPProtocol { // will be handled by udp proxy } else { - //nolint:ineffassign isFailed = true - // set status as failed and exit - _ = ingressRule.UpdateStatus(ctx, dbWithoutTx, core.IngressRuleStatusFailed) - // no requeue - return nil + break } } @@ -158,12 +134,8 @@ func (m Manager) IngressRuleApply(request IngressRuleApplyRequest, ctx context.C Service: application.Name, }, restrictedPorts) if err != nil { - //nolint:ineffassign isFailed = true - // set status as failed and exit - _ = ingressRule.UpdateStatus(ctx, dbWithoutTx, core.IngressRuleStatusFailed) - // no requeue - return nil + break } } } @@ -174,6 +146,7 @@ func (m Manager) IngressRuleApply(request IngressRuleApplyRequest, ctx context.C err = haproxyManager.CommitTransaction(haproxyTransactionId) } if isFailed || err != nil { + isFailed = true log.Println("failed to commit haproxy transaction", err) err := haproxyManager.DeleteTransaction(haproxyTransactionId) if err != nil { @@ -183,19 +156,9 @@ func (m Manager) IngressRuleApply(request IngressRuleApplyRequest, ctx context.C } if isFailed { - // set status as failed and exit - _ = ingressRule.UpdateStatus(ctx, dbWithoutTx, core.IngressRuleStatusFailed) - // no requeue - return nil + return ingressRule.UpdateStatus(ctx, dbWithoutTx, core.IngressRuleStatusFailed) + } else { + // update status as applied + return ingressRule.UpdateStatus(ctx, dbWithoutTx, core.IngressRuleStatusApplied) } - - // update status as applied - err = ingressRule.UpdateStatus(ctx, dbWithoutTx, core.IngressRuleStatusApplied) - if err != nil { - // requeue because this error can lead to block stage of application - return err - } - - // success - return nil } diff --git a/swiftwave_service/worker/process_ingress_rule_delete_request.go b/swiftwave_service/worker/process_ingress_rule_delete_request.go index 918ec08749..2d742ebf62 100644 --- a/swiftwave_service/worker/process_ingress_rule_delete_request.go +++ b/swiftwave_service/worker/process_ingress_rule_delete_request.go @@ -92,10 +92,8 @@ func (m Manager) IngressRuleDelete(request IngressRuleDeleteRequest, ctx context if err != nil { // set status as failed and exit // because `DeleteHTTPSLink` can fail only if haproxy not working - //nolint:ineffassign isFailed = true - // requeue required as it fault of haproxy and may be resolved in next try - return err + break } } else if ingressRule.Protocol == core.HTTPProtocol { if ingressRule.Port == 80 { @@ -103,20 +101,16 @@ func (m Manager) IngressRuleDelete(request IngressRuleDeleteRequest, ctx context if err != nil { // set status as failed and exit // because `DeleteHTTPLink` can fail only if haproxy not working - //nolint:ineffassign isFailed = true - // requeue required as it fault of haproxy and may be resolved in next try - return err + break } } else { err = haproxyManager.DeleteTCPLink(haproxyTransactionId, backendName, int(ingressRule.Port), domain.Name, haproxymanager.HTTPMode) if err != nil { // set status as failed and exit // because `DeleteTCPLink` can fail only if haproxy not working - //nolint:ineffassign isFailed = true - // requeue required as it fault of haproxy and may be resolved in next try - return err + break } } } else if ingressRule.Protocol == core.TCPProtocol { @@ -125,17 +119,13 @@ func (m Manager) IngressRuleDelete(request IngressRuleDeleteRequest, ctx context if err != nil { // set status as failed and exit // because `DeleteTCPLink` can fail only if haproxy not working - //nolint:ineffassign isFailed = true - // requeue required as it fault of haproxy and may be resolved in next try - return err + break } } else if ingressRule.Protocol == core.UDPProtocol { // leave it for udp proxy } else { // unknown protocol - //nolint:ineffassign - isFailed = true return nil } @@ -153,10 +143,8 @@ func (m Manager) IngressRuleDelete(request IngressRuleDeleteRequest, ctx context if err != nil { // set status as failed and exit // because `DeleteBackend` can fail only if haproxy not working - //nolint:ineffassign isFailed = true - // requeue required as it fault of haproxy and may be resolved in next try - return err + break } } } @@ -171,10 +159,8 @@ func (m Manager) IngressRuleDelete(request IngressRuleDeleteRequest, ctx context }) if err != nil { // set status as failed and exit - //nolint:ineffassign isFailed = true - // requeue required as it fault of udp proxy and may be resolved in next try - return err + break } } } @@ -188,6 +174,7 @@ func (m Manager) IngressRuleDelete(request IngressRuleDeleteRequest, ctx context } } if isFailed || err != nil { + isFailed = true log.Println("failed to commit haproxy transaction", err) err := haproxyManager.DeleteTransaction(haproxyTransactionId) if err != nil { @@ -196,11 +183,10 @@ func (m Manager) IngressRuleDelete(request IngressRuleDeleteRequest, ctx context } } - // delete ingress rule from database - err = ingressRule.Delete(ctx, dbWithoutTx, true) - if err != nil { - return err + if isFailed { + return ingressRule.UpdateStatus(ctx, dbWithoutTx, core.IngressRuleStatusFailed) + } else { + // delete ingress rule from database + return ingressRule.Delete(ctx, dbWithoutTx, true) } - - return nil } diff --git a/swiftwave_service/worker/process_install_dependencies_on_server_request.go b/swiftwave_service/worker/process_install_dependencies_on_server_request.go index 430ede648a..535d474b6f 100644 --- a/swiftwave_service/worker/process_install_dependencies_on_server_request.go +++ b/swiftwave_service/worker/process_install_dependencies_on_server_request.go @@ -7,7 +7,6 @@ import ( "github.com/swiftwave-org/swiftwave/ssh_toolkit" "github.com/swiftwave-org/swiftwave/swiftwave_service/core" "gorm.io/gorm" - "strings" "time" ) @@ -60,20 +59,15 @@ func (m Manager) InstallDependenciesOnServer(request InstallDependenciesOnServer // command var command string - var isExists bool for _, dependency := range core.RequiredServerDependencies { - isExists = false + isExists := false // check if dependency is already installed [ignore init] if dependency != "init" { stdoutBuffer := new(bytes.Buffer) err = ssh_toolkit.ExecCommandOverSSH(core.DependencyCheckCommands[dependency], stdoutBuffer, nil, 5, server.IP, 22, server.User, m.Config.SystemConfig.SshPrivateKey, 30) - if err != nil { - if strings.Contains(err.Error(), "exited with status 1") { - //nolint:ineffassign - isExists = false - } + if err == nil { + isExists = stdoutBuffer.String() != "" } - isExists = stdoutBuffer.String() != "" } // install dependency if isExists { diff --git a/swiftwave_service/worker/process_redirect_rule_apply_request.go b/swiftwave_service/worker/process_redirect_rule_apply_request.go index 5b9d27ce78..a63a616f15 100644 --- a/swiftwave_service/worker/process_redirect_rule_apply_request.go +++ b/swiftwave_service/worker/process_redirect_rule_apply_request.go @@ -66,22 +66,8 @@ func (m Manager) RedirectRuleApply(request RedirectRuleApplyRequest, ctx context err = haproxyManager.AddHTTPSRedirectRule(haproxyTransactionId, domain.Name, redirectRule.RedirectURL) } if err != nil { - // set status as failed and exit - _ = redirectRule.UpdateStatus(ctx, dbWithoutTx, core.RedirectRuleStatusFailed) - //nolint:ineffassign isFailed = true - // no requeue - return nil - } - // commit haproxy transaction - err = haproxyManager.CommitTransaction(haproxyTransactionId) - if err != nil { - // set status as failed and exit - _ = redirectRule.UpdateStatus(ctx, dbWithoutTx, core.RedirectRuleStatusFailed) - //nolint:ineffassign - isFailed = true - // no requeue - return nil + break } } @@ -91,6 +77,7 @@ func (m Manager) RedirectRuleApply(request RedirectRuleApplyRequest, ctx context err = haproxyManager.CommitTransaction(haproxyTransactionId) } if isFailed || err != nil { + isFailed = true log.Println("failed to commit haproxy transaction", err) err := haproxyManager.DeleteTransaction(haproxyTransactionId) if err != nil { @@ -99,11 +86,10 @@ func (m Manager) RedirectRuleApply(request RedirectRuleApplyRequest, ctx context } } - // set status as applied - err = redirectRule.UpdateStatus(ctx, dbWithoutTx, core.RedirectRuleStatusApplied) - if err != nil { - return err + // set status + if isFailed { + return redirectRule.UpdateStatus(ctx, dbWithoutTx, core.RedirectRuleStatusFailed) + } else { + return redirectRule.UpdateStatus(ctx, dbWithoutTx, core.RedirectRuleStatusApplied) } - - return nil } diff --git a/swiftwave_service/worker/process_redirect_rule_delete_request.go b/swiftwave_service/worker/process_redirect_rule_delete_request.go index 9d590f72db..234096ff36 100644 --- a/swiftwave_service/worker/process_redirect_rule_delete_request.go +++ b/swiftwave_service/worker/process_redirect_rule_delete_request.go @@ -71,12 +71,8 @@ func (m Manager) RedirectRuleDelete(request RedirectRuleDeleteRequest, ctx conte return nil } if err != nil { - // set status as failed and exit - // because `DeleteHTTPRedirectRule` can fail only if haproxy not working - //nolint:ineffassign isFailed = true - // requeue required as it fault of haproxy and may be resolved in next try - return err + break } } @@ -86,6 +82,7 @@ func (m Manager) RedirectRuleDelete(request RedirectRuleDeleteRequest, ctx conte err = haproxyManager.CommitTransaction(haproxyTransactionId) } if isFailed || err != nil { + isFailed = true log.Println("failed to commit haproxy transaction", err) err := haproxyManager.DeleteTransaction(haproxyTransactionId) if err != nil { @@ -94,10 +91,12 @@ func (m Manager) RedirectRuleDelete(request RedirectRuleDeleteRequest, ctx conte } } - // delete redirect rule from database - err = redirectRule.Delete(ctx, dbWithoutTx, true) - if err != nil { - return err + if !isFailed { + // delete redirect rule from database + _ = redirectRule.Delete(ctx, dbWithoutTx, true) + return nil + } else { + // update status + return redirectRule.UpdateStatus(ctx, dbWithoutTx, core.RedirectRuleStatusFailed) } - return nil } diff --git a/swiftwave_service/worker/process_ssl_request.go b/swiftwave_service/worker/process_ssl_request.go index 6646a25bdd..ef947b6c35 100644 --- a/swiftwave_service/worker/process_ssl_request.go +++ b/swiftwave_service/worker/process_ssl_request.go @@ -94,9 +94,8 @@ func (m Manager) SSLGenerate(request SSLGenerateRequest, ctx context.Context, _ // upload certificate to haproxy err = haproxyManager.UpdateSSL(transactionId, domain.Name, []byte(domain.SSLPrivateKey), []byte(domain.SSLFullChain)) if err != nil { - //nolint:ineffassign isFailed = true - return err + break } } for haproxyManager, haproxyTransactionId := range transactionIdMap { @@ -105,6 +104,7 @@ func (m Manager) SSLGenerate(request SSLGenerateRequest, ctx context.Context, _ err = haproxyManager.CommitTransaction(haproxyTransactionId) } if isFailed || err != nil { + isFailed = true log.Println("failed to commit haproxy transaction", err) err := haproxyManager.DeleteTransaction(haproxyTransactionId) if err != nil { @@ -112,6 +112,11 @@ func (m Manager) SSLGenerate(request SSLGenerateRequest, ctx context.Context, _ } } } + + if isFailed { + return domain.UpdateSSLStatus(ctx, dbWithoutTx, core.DomainSSLStatusFailed) + } + return nil }