From 109395e0a5a667cd159ae822b57d5631fc21ca38 Mon Sep 17 00:00:00 2001 From: Maysun Faisal <31771087+maysunfaisal@users.noreply.github.com> Date: Wed, 13 Dec 2023 18:03:43 -0500 Subject: [PATCH] Add prometheus metrics for Component creation (#431) Signed-off-by: Maysun J Faisal --- controllers/component_controller.go | 108 +++++++++- controllers/component_controller_test.go | 202 ++++++++++++++++++ controllers/component_controller_unit_test.go | 84 ++++++++ controllers/errors.go | 8 + controllers/update.go | 12 +- pkg/devfile/devfile.go | 36 ++-- pkg/devfile/errors.go | 20 ++ pkg/github/github.go | 3 + pkg/github/github_test.go | 15 ++ pkg/metrics/metrics.go | 29 ++- 10 files changed, 483 insertions(+), 34 deletions(-) diff --git a/controllers/component_controller.go b/controllers/component_controller.go index 52918ec6..bd1c0365 100644 --- a/controllers/component_controller.go +++ b/controllers/component_controller.go @@ -217,9 +217,14 @@ func (r *ComponentReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( log.Info(fmt.Sprintf("Starting reconcile loop for %v", req.NamespacedName)) + isCreateReconcile := checkForCreateReconcile(component) + if isCreateReconcile { + metrics.ComponentCreationTotalReqs.Inc() + } + // Check if GitOps generation has failed on a reconcile // Attempt to generate GitOps and set appropriate conditions accordingly - isUpdateConditionPresent := false + isUpdateErrConditionPresent := false isGitOpsRegenSuccessful := false for _, condition := range component.Status.Conditions { if forceGenerateGitopsResource || (condition.Type == "GitOpsResourcesGenerated" && condition.Reason == "GenerateError" && condition.Status == metav1.ConditionFalse) { @@ -230,6 +235,10 @@ func (r *ComponentReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( // 2. Kubernetes Component Uri has already been converted to inlined content with a Token if required by default on the first parse compDevfileData, err := cdqanalysis.ParseDevfileWithParserArgs(&devfileParser.ParserArgs{Data: []byte(component.Status.Devfile)}) if err != nil { + if isCreateReconcile { + // Gate it with a Create reconcile flag check since this code is executed by both Create and Update reconciliation + metrics.ComponentCreationFailed.Inc() + } errMsg := fmt.Sprintf("Unable to parse the devfile from Component status and re-attempt GitOps generation, exiting reconcile loop %v", req.NamespacedName) log.Error(err, errMsg) _ = r.SetGitOpsGeneratedConditionAndUpdateCR(ctx, req, &component, fmt.Errorf("%v: %v", errMsg, err)) @@ -250,11 +259,11 @@ func (r *ComponentReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( isGitOpsRegenSuccessful = true } } else if condition.Type == "Updated" && condition.Reason == "Error" && condition.Status == metav1.ConditionFalse { - isUpdateConditionPresent = true + isUpdateErrConditionPresent = true } } - if isGitOpsRegenSuccessful && isUpdateConditionPresent { + if isGitOpsRegenSuccessful && isUpdateErrConditionPresent { err = r.SetUpdateConditionAndUpdateCR(ctx, req, &component, nil) if err != nil { return ctrl.Result{}, err @@ -265,6 +274,7 @@ func (r *ComponentReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( if err != nil { return ctrl.Result{}, err } + metrics.ComponentCreationSucceeded.Inc() return ctrl.Result{}, nil } @@ -319,6 +329,7 @@ func (r *ComponentReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( if err != nil { // ConvertGitHubURL only returns user error metrics.ImportGitRepoSucceeded.Inc() + metrics.ComponentCreationSucceeded.Inc() log.Error(err, fmt.Sprintf("Unable to convert Github URL to raw format, exiting reconcile loop %v", req.NamespacedName)) _ = r.SetCreateConditionAndUpdateCR(ctx, req, &component, err) return ctrl.Result{}, err @@ -328,6 +339,7 @@ func (r *ComponentReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( // FindAndDownloadDevfile only returns user error metrics.ImportGitRepoSucceeded.Inc() if err != nil { + metrics.ComponentCreationSucceeded.Inc() log.Error(err, fmt.Sprintf("Unable to read the devfile from dir %s %v", gitURL, req.NamespacedName)) _ = r.SetCreateConditionAndUpdateCR(ctx, req, &component, err) return ctrl.Result{}, err @@ -340,11 +352,13 @@ func (r *ComponentReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( // Use SPI to retrieve the devfile from the private repository devfileBytes, devfileLocation, err = spi.DownloadDevfileUsingSPI(r.SPIClient, ctx, component, gitURL, source.GitSource.Revision, context) if err != nil { - // Increment the import git repo failed metric on non-user errors - if _, ok := err.(*devfile.NoFileFound); !ok { + // Increment the import git repo and component create failed metric on non-user errors + if _, ok := err.(*cdqanalysis.NoDevfileFound); !ok { metrics.ImportGitRepoFailed.Inc() + metrics.ComponentCreationFailed.Inc() } else { metrics.ImportGitRepoSucceeded.Inc() + metrics.ComponentCreationSucceeded.Inc() } log.Error(err, fmt.Sprintf("Unable to download from any known devfile locations from %s %v", gitURL, req.NamespacedName)) _ = r.SetCreateConditionAndUpdateCR(ctx, req, &component, err) @@ -354,6 +368,8 @@ func (r *ComponentReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( gitURL, err := cdqanalysis.ConvertGitHubURL(source.GitSource.URL, source.GitSource.Revision, context) if err != nil { + // User error - so increment the "success" metric - since we're tracking only system errors + metrics.ComponentCreationSucceeded.Inc() log.Error(err, fmt.Sprintf("Unable to convert Github URL to raw format, exiting reconcile loop %v", req.NamespacedName)) _ = r.SetCreateConditionAndUpdateCR(ctx, req, &component, err) return ctrl.Result{}, err @@ -366,6 +382,7 @@ func (r *ComponentReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( } else if source.GitSource.DockerfileURL != "" { devfileData, err := devfile.CreateDevfileForDockerfileBuild(source.GitSource.DockerfileURL, "./", component.Name, component.Spec.Application) if err != nil { + metrics.ComponentCreationFailed.Inc() log.Error(err, fmt.Sprintf("Unable to create devfile for Dockerfile build %v", req.NamespacedName)) _ = r.SetCreateConditionAndUpdateCR(ctx, req, &component, err) return ctrl.Result{}, err @@ -373,6 +390,7 @@ func (r *ComponentReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( devfileBytes, err = yaml.Marshal(devfileData) if err != nil { + metrics.ComponentCreationFailed.Inc() log.Error(err, fmt.Sprintf("Unable to marshal devfile, exiting reconcile loop %v", req.NamespacedName)) err = r.SetCreateConditionAndUpdateCR(ctx, req, &component, err) if err != nil { @@ -386,6 +404,7 @@ func (r *ComponentReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( // Generate a stub devfile for the component devfileData, err := devfile.ConvertImageComponentToDevfile(component) if err != nil { + metrics.ComponentCreationFailed.Inc() log.Error(err, fmt.Sprintf("Unable to convert the Image Component to a devfile %v", req.NamespacedName)) _ = r.SetCreateConditionAndUpdateCR(ctx, req, &component, err) return ctrl.Result{}, err @@ -394,6 +413,7 @@ func (r *ComponentReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( devfileBytes, err = yaml.Marshal(devfileData) if err != nil { + metrics.ComponentCreationFailed.Inc() log.Error(err, fmt.Sprintf("Unable to marshal devfile, exiting reconcile loop %v", req.NamespacedName)) err = r.SetCreateConditionAndUpdateCR(ctx, req, &component, err) if err != nil { @@ -413,6 +433,10 @@ func (r *ComponentReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( compDevfileData, err = cdqanalysis.ParseDevfileWithParserArgs(&devfileParser.ParserArgs{URL: devfileLocation, Token: gitToken, DevfileUtilsClient: r.DevfileUtilsClient}) if err != nil { + // Even though we are passing in a git token to devfile/library and there is a possibility of a bad token usage, + // we consider this a system error because the git token is being used by the git client to fetch the branch names, + // if there was an issue with the token, it would have been surfaced by the github client previously. + metrics.ComponentCreationFailed.Inc() log.Error(err, fmt.Sprintf("Unable to parse the devfile from Component devfile location, exiting reconcile loop %v", req.NamespacedName)) _ = r.SetCreateConditionAndUpdateCR(ctx, req, &component, err) return ctrl.Result{}, err @@ -426,6 +450,7 @@ func (r *ComponentReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( compDevfileData, err = cdqanalysis.ParseDevfileWithParserArgs(&devfileParser.ParserArgs{Data: devfileBytes}) if err != nil { + metrics.ComponentCreationFailed.Inc() log.Error(err, fmt.Sprintf("Unable to parse the devfile from Component, exiting reconcile loop %v", req.NamespacedName)) _ = r.SetCreateConditionAndUpdateCR(ctx, req, &component, err) return ctrl.Result{}, err @@ -434,6 +459,14 @@ func (r *ComponentReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( err = r.updateComponentDevfileModel(req, compDevfileData, component) if err != nil { + // Increment the Component create failed metric on non-user errors + if _, ok := err.(*NotSupported); ok { + metrics.ComponentCreationSucceeded.Inc() + } else if _, ok := err.(*devfile.DevfileAttributeParse); ok { + metrics.ComponentCreationSucceeded.Inc() + } else { + metrics.ComponentCreationFailed.Inc() + } log.Error(err, fmt.Sprintf("Unable to update the Component Devfile model %v", req.NamespacedName)) _ = r.SetCreateConditionAndUpdateCR(ctx, req, &component, err) return ctrl.Result{}, err @@ -447,6 +480,7 @@ func (r *ComponentReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( hasAppDevfileData, err := cdqanalysis.ParseDevfileWithParserArgs(&devfileParser.ParserArgs{Data: []byte(hasApplication.Status.Devfile)}) if err != nil { + metrics.ComponentCreationFailed.Inc() log.Error(err, fmt.Sprintf("Unable to parse the devfile from Application, exiting reconcile loop %v", req.NamespacedName)) _ = r.SetCreateConditionAndUpdateCR(ctx, req, &component, err) return ctrl.Result{}, err @@ -454,6 +488,7 @@ func (r *ComponentReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( yamlHASCompData, err := yaml.Marshal(compDevfileData) if err != nil { + metrics.ComponentCreationFailed.Inc() log.Error(err, fmt.Sprintf("Unable to marshall the Component devfile, exiting reconcile loop %v", req.NamespacedName)) _ = r.SetCreateConditionAndUpdateCR(ctx, req, &component, err) return ctrl.Result{}, err @@ -467,6 +502,7 @@ func (r *ComponentReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( log.Info(fmt.Sprintf("Adding the GitOps repository information to the status for component %v", req.NamespacedName)) err = setGitopsStatus(&component, hasAppDevfileData) if err != nil { + metrics.ComponentCreationFailed.Inc() log.Error(err, fmt.Sprintf("Unable to retrieve gitops repository information for resource %v", req.NamespacedName)) _ = r.SetCreateConditionAndUpdateCR(ctx, req, &component, err) return ctrl.Result{}, err @@ -492,6 +528,7 @@ func (r *ComponentReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( if err != nil { return ctrl.Result{}, err } + metrics.ComponentCreationSucceeded.Inc() } } else { @@ -599,14 +636,22 @@ func (r *ComponentReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( func (r *ComponentReconciler) generateGitops(ctx context.Context, ghClient *github.GitHubClient, component *appstudiov1alpha1.Component, compDevfileData data.DevfileData) error { log := ctrl.LoggerFrom(ctx) + isCreateReconcile := checkForCreateReconcile(*component) + gitOpsURL, gitOpsBranch, gitOpsContext, err := util.ProcessGitOpsStatus(component.Status.GitOps, ghClient.Token) if err != nil { + if isCreateReconcile { + metrics.ComponentCreationFailed.Inc() + } return err } // Create a temp folder to create the gitops resources in tempDir, err := ioutils.CreateTempPath(component.Name, r.AppFS) if err != nil { + if isCreateReconcile { + metrics.ComponentCreationFailed.Inc() + } log.Error(err, "unable to create temp directory for GitOps resources due to error") ioutils.RemoveFolderAndLogError(log, r.AppFS, tempDir) return fmt.Errorf("unable to create temp directory for GitOps resources due to error: %v", err) @@ -614,6 +659,9 @@ func (r *ComponentReconciler) generateGitops(ctx context.Context, ghClient *gith deployAssociatedComponents, err := devfileParser.GetDeployComponents(compDevfileData) if err != nil { + if isCreateReconcile { + metrics.ComponentCreationFailed.Inc() + } log.Error(err, "unable to get deploy components") ioutils.RemoveFolderAndLogError(log, r.AppFS, tempDir) return err @@ -621,6 +669,15 @@ func (r *ComponentReconciler) generateGitops(ctx context.Context, ghClient *gith kubernetesResources, err := devfile.GetResourceFromDevfile(log, compDevfileData, deployAssociatedComponents, component.Name, component.Spec.Application, component.Spec.ContainerImage, "") if err != nil { + if _, ok := err.(*devfile.DevfileAttributeParse); ok && isCreateReconcile { + // Attribute parse error from Devfile is considered an user error + metrics.ComponentCreationSucceeded.Inc() + } else if _, ok := err.(*devfile.MissingOuterloop); ok && isCreateReconcile { + // If Devfile has no Outerloop component, it is considered an user error + metrics.ComponentCreationSucceeded.Inc() + } else if isCreateReconcile { + metrics.ComponentCreationFailed.Inc() + } log.Error(err, "unable to get kubernetes resources from the devfile outerloop components") ioutils.RemoveFolderAndLogError(log, r.AppFS, tempDir) return err @@ -645,7 +702,14 @@ func (r *ComponentReconciler) generateGitops(ctx context.Context, ghClient *gith unblockURL = fmt.Sprintf("%v/security/secret-scanning/unblock-secret/%v", component.Status.GitOps.RepositoryURL, token) log.Error(retErr, fmt.Sprintf("unable to generate gitops resources due to git push protecton error, follow the link to unblock the secret: %v", unblockURL)) } + if isCreateReconcile { + // Secret leak error is considered an user error + metrics.ComponentCreationSucceeded.Inc() + } } else { + if isCreateReconcile { + metrics.ComponentCreationFailed.Inc() + } log.Error(retErr, "unable to generate gitops resources due to error") } ioutils.RemoveFolderAndLogError(log, r.AppFS, tempDir) @@ -668,7 +732,14 @@ func (r *ComponentReconciler) generateGitops(ctx context.Context, ghClient *gith unblockURL = fmt.Sprintf("%v/security/secret-scanning/unblock-secret/%v", component.Status.GitOps.RepositoryURL, token) log.Error(retErr, fmt.Sprintf("unable to commit and push gitops resources due to git push protecton error, follow the link to unblock the secret: %v", unblockURL)) } + if isCreateReconcile { + // Secret leak error is considered an user error + metrics.ComponentCreationSucceeded.Inc() + } } else { + if isCreateReconcile { + metrics.ComponentCreationFailed.Inc() + } log.Error(retErr, "unable to commit and push gitops resources due to error") } ioutils.RemoveFolderAndLogError(log, r.AppFS, tempDir) @@ -681,6 +752,9 @@ func (r *ComponentReconciler) generateGitops(ctx context.Context, ghClient *gith metricsLabel := prometheus.Labels{"controller": componentName, "tokenName": ghClient.TokenName, "operation": "GetCommitIDFromRepo"} metrics.ControllerGitRequest.With(metricsLabel).Inc() if commitID, err = r.Generator.GetCommitIDFromRepo(r.AppFS, repoPath); err != nil { + if isCreateReconcile { + metrics.ComponentCreationFailed.Inc() + } log.Error(err, "") ioutils.RemoveFolderAndLogError(log, r.AppFS, tempDir) return err @@ -689,7 +763,16 @@ func (r *ComponentReconciler) generateGitops(ctx context.Context, ghClient *gith component.Status.GitOps.CommitID = commitID // Remove the temp folder that was created - return r.AppFS.RemoveAll(tempDir) + err = r.AppFS.RemoveAll(tempDir) + if err != nil { + if isCreateReconcile { + metrics.ComponentCreationFailed.Inc() + } + log.Error(err, "unable to remove temp dir") + return err + } + + return nil } // setGitopsStatus adds the necessary gitops info (url, branch, context) to the component CR status @@ -796,3 +879,16 @@ func setForceGenerateGitopsAnnotation(component *appstudiov1alpha1.Component, va } component.Annotations[forceGenerationAnnotation] = value } + +func checkForCreateReconcile(component appstudiov1alpha1.Component) bool { + // Determine if this is a Create reconcile or an Update reconcile based on Conditions + for _, condition := range component.Status.Conditions { + if condition.Type == "Updated" { + // If an Updated Condition is present, it means this is an Update reconcile + return false + } + } + + // If there are no Conditions or no Updated Condition, then this is a Create reconcile + return true +} diff --git a/controllers/component_controller_test.go b/controllers/component_controller_test.go index 39140576..36e55c18 100644 --- a/controllers/component_controller_test.go +++ b/controllers/component_controller_test.go @@ -22,6 +22,7 @@ import ( "strings" "testing" + "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/testutil" "github.com/redhat-appstudio/application-service/pkg/metrics" @@ -61,8 +62,14 @@ var _ = Describe("Component controller", func() { gitToken = "" //empty for public repo test ) + prometheus.MustRegister(metrics.ComponentCreationTotalReqs, metrics.ComponentCreationFailed, metrics.ComponentCreationSucceeded) + Context("Create Component with basic field set", func() { It("Should create successfully and update the Application", func() { + beforeCreateTotalReqs := testutil.ToFloat64(metrics.ComponentCreationTotalReqs) + beforeCreateSucceedReqs := testutil.ToFloat64(metrics.ComponentCreationSucceeded) + beforeCreateFailedReqs := testutil.ToFloat64(metrics.ComponentCreationFailed) + ctx := context.Background() applicationName := HASAppName + "1" @@ -159,6 +166,10 @@ var _ = Describe("Component controller", func() { Expect(nameMatched).Should(Equal(true)) Expect(repoLinkMatched).Should(Equal(true)) + Expect(testutil.ToFloat64(metrics.ComponentCreationTotalReqs) > beforeCreateTotalReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationSucceeded) > beforeCreateSucceedReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationFailed) == beforeCreateFailedReqs).To(BeTrue()) + // Delete the specified HASComp resource deleteHASCompCR(hasCompLookupKey) @@ -169,6 +180,10 @@ var _ = Describe("Component controller", func() { Context("Create Component with basic field set including devfileURL", func() { It("Should create successfully on a valid url", func() { + beforeCreateTotalReqs := testutil.ToFloat64(metrics.ComponentCreationTotalReqs) + beforeCreateSucceedReqs := testutil.ToFloat64(metrics.ComponentCreationSucceeded) + beforeCreateFailedReqs := testutil.ToFloat64(metrics.ComponentCreationFailed) + ctx := context.Background() applicationName := HASAppName + "2" @@ -256,6 +271,10 @@ var _ = Describe("Component controller", func() { Expect(nameMatched).Should(Equal(true)) Expect(repoLinkMatched).Should(Equal(true)) + Expect(testutil.ToFloat64(metrics.ComponentCreationTotalReqs) > beforeCreateTotalReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationSucceeded) > beforeCreateSucceedReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationFailed) == beforeCreateFailedReqs).To(BeTrue()) + // Delete the specified HASComp resource deleteHASCompCR(hasCompLookupKey) @@ -266,6 +285,10 @@ var _ = Describe("Component controller", func() { Context("Create Component with basic field set including devfileURL", func() { It("Should error out on a bad url", func() { + beforeCreateTotalReqs := testutil.ToFloat64(metrics.ComponentCreationTotalReqs) + beforeCreateSucceedReqs := testutil.ToFloat64(metrics.ComponentCreationSucceeded) + beforeCreateFailedReqs := testutil.ToFloat64(metrics.ComponentCreationFailed) + ctx := context.Background() applicationName := HASAppName + "3" @@ -313,6 +336,10 @@ var _ = Describe("Component controller", func() { hasAppLookupKey := types.NamespacedName{Name: applicationName, Namespace: HASAppNamespace} + Expect(testutil.ToFloat64(metrics.ComponentCreationTotalReqs) > beforeCreateTotalReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationSucceeded) == beforeCreateSucceedReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationFailed) > beforeCreateFailedReqs).To(BeTrue()) + // Delete the specified HASComp resource deleteHASCompCR(hasCompLookupKey) @@ -323,6 +350,10 @@ var _ = Describe("Component controller", func() { Context("Create a Component before an Application", func() { It("Should reconcile once the application is created", func() { + beforeCreateTotalReqs := testutil.ToFloat64(metrics.ComponentCreationTotalReqs) + beforeCreateSucceedReqs := testutil.ToFloat64(metrics.ComponentCreationSucceeded) + beforeCreateFailedReqs := testutil.ToFloat64(metrics.ComponentCreationFailed) + ctx := context.Background() applicationName := HASAppName + "4" @@ -378,6 +409,10 @@ var _ = Describe("Component controller", func() { return len(createdHasComp.Status.Conditions) > 0 && createdHasComp.Status.Conditions[0].Reason == "OK" }, timeout40s, interval).Should(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationTotalReqs) > beforeCreateTotalReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationSucceeded) > beforeCreateSucceedReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationFailed) == beforeCreateFailedReqs).To(BeTrue()) + // Delete the specified HASComp resource deleteHASAppCR(types.NamespacedName{Name: applicationName, Namespace: HASAppNamespace}) deleteHASCompCR(hasCompLookupKey) @@ -387,6 +422,10 @@ var _ = Describe("Component controller", func() { Context("Create Component with other field set", func() { It("Should create successfully and update the Application", func() { + beforeCreateTotalReqs := testutil.ToFloat64(metrics.ComponentCreationTotalReqs) + beforeCreateSucceedReqs := testutil.ToFloat64(metrics.ComponentCreationSucceeded) + beforeCreateFailedReqs := testutil.ToFloat64(metrics.ComponentCreationFailed) + ctx := context.Background() applicationName := HASAppName + "5" @@ -593,6 +632,10 @@ var _ = Describe("Component controller", func() { verifyHASComponentUpdates(hasCompUpdatedDevfile, checklist, nil) + Expect(testutil.ToFloat64(metrics.ComponentCreationTotalReqs) > beforeCreateTotalReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationSucceeded) > beforeCreateSucceedReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationFailed) == beforeCreateFailedReqs).To(BeTrue()) + // Delete the specified HASComp resource deleteHASCompCR(hasCompLookupKey) @@ -603,6 +646,10 @@ var _ = Describe("Component controller", func() { Context("Create Component with built container image set", func() { It("Should create successfully", func() { + beforeCreateTotalReqs := testutil.ToFloat64(metrics.ComponentCreationTotalReqs) + beforeCreateSucceedReqs := testutil.ToFloat64(metrics.ComponentCreationSucceeded) + beforeCreateFailedReqs := testutil.ToFloat64(metrics.ComponentCreationFailed) + ctx := context.Background() beforeImportGitRepoTotalReqs := testutil.ToFloat64(metrics.ImportGitRepoTotalReqs) @@ -672,6 +719,10 @@ var _ = Describe("Component controller", func() { Expect(testutil.ToFloat64(metrics.ImportGitRepoSucceeded) > beforeImportGitRepoSucceeded).To(BeTrue()) Expect(testutil.ToFloat64(metrics.ImportGitRepoFailed) == beforeImportGitRepoFailed).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationTotalReqs) > beforeCreateTotalReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationSucceeded) > beforeCreateSucceedReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationFailed) == beforeCreateFailedReqs).To(BeTrue()) + // Delete the specified HASComp resource deleteHASCompCR(hasCompLookupKey) @@ -682,6 +733,10 @@ var _ = Describe("Component controller", func() { Context("Component with invalid devfile", func() { It("Should fail and have proper failure condition set", func() { + beforeCreateTotalReqs := testutil.ToFloat64(metrics.ComponentCreationTotalReqs) + beforeCreateSucceedReqs := testutil.ToFloat64(metrics.ComponentCreationSucceeded) + beforeCreateFailedReqs := testutil.ToFloat64(metrics.ComponentCreationFailed) + ctx := context.Background() applicationName := HASAppName + "7" @@ -739,6 +794,10 @@ var _ = Describe("Component controller", func() { Expect(errCondition.Status).Should(Equal(metav1.ConditionFalse)) Expect(errCondition.Message).Should(ContainSubstring("failed to decode devfile json")) + Expect(testutil.ToFloat64(metrics.ComponentCreationTotalReqs) > beforeCreateTotalReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationSucceeded) > beforeCreateSucceedReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationFailed) == beforeCreateFailedReqs).To(BeTrue()) + // Delete the specified HASComp resource deleteHASCompCR(hasCompLookupKey) @@ -756,6 +815,10 @@ var _ = Describe("Component controller", func() { // This first test will just use an invalid gitops repository url for the component Context("Component with gitops resource generation failure", func() { It("Should have proper failure condition set", func() { + beforeCreateTotalReqs := testutil.ToFloat64(metrics.ComponentCreationTotalReqs) + beforeCreateSucceedReqs := testutil.ToFloat64(metrics.ComponentCreationSucceeded) + beforeCreateFailedReqs := testutil.ToFloat64(metrics.ComponentCreationFailed) + ctx := context.Background() applicationName := HASAppName + "8" @@ -829,6 +892,10 @@ var _ = Describe("Component controller", func() { Expect(gitopsCondition.Type).To(Equal("GitOpsResourcesGenerated")) Expect(gitopsCondition.Status).To(Equal(metav1.ConditionFalse)) + Expect(testutil.ToFloat64(metrics.ComponentCreationTotalReqs) > beforeCreateTotalReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationSucceeded) == beforeCreateSucceedReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationFailed) > beforeCreateFailedReqs).To(BeTrue()) + // Delete the specified HASComp resource deleteHASCompCR(hasCompLookupKey) @@ -910,6 +977,10 @@ var _ = Describe("Component controller", func() { // The gitops generation should fail due to the gitops repository annotation missing Context("Component created with App with missing gitops repository", func() { It("Should fail since Application has no gitops repository", func() { + beforeCreateTotalReqs := testutil.ToFloat64(metrics.ComponentCreationTotalReqs) + beforeCreateSucceedReqs := testutil.ToFloat64(metrics.ComponentCreationSucceeded) + beforeCreateFailedReqs := testutil.ToFloat64(metrics.ComponentCreationFailed) + var err error ctx := context.Background() @@ -976,6 +1047,10 @@ var _ = Describe("Component controller", func() { Expect(createdHasComp.Status.Conditions[len(createdHasComp.Status.Conditions)-1].Message).Should(ContainSubstring("unable to retrieve GitOps repository from Application CR devfile")) + Expect(testutil.ToFloat64(metrics.ComponentCreationTotalReqs) > beforeCreateTotalReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationSucceeded) == beforeCreateSucceedReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationFailed) > beforeCreateFailedReqs).To(BeTrue()) + // Delete the specified HASComp resource deleteHASCompCR(hasCompLookupKey) @@ -986,6 +1061,10 @@ var _ = Describe("Component controller", func() { Context("Create Component with invalid git url", func() { It("Should fail with error", func() { + beforeCreateTotalReqs := testutil.ToFloat64(metrics.ComponentCreationTotalReqs) + beforeCreateSucceedReqs := testutil.ToFloat64(metrics.ComponentCreationSucceeded) + beforeCreateFailedReqs := testutil.ToFloat64(metrics.ComponentCreationFailed) + ctx := context.Background() applicationName := HASAppName + "11" @@ -1032,6 +1111,10 @@ var _ = Describe("Component controller", func() { hasAppLookupKey := types.NamespacedName{Name: applicationName, Namespace: HASAppNamespace} + Expect(testutil.ToFloat64(metrics.ComponentCreationTotalReqs) > beforeCreateTotalReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationSucceeded) > beforeCreateSucceedReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationFailed) == beforeCreateFailedReqs).To(BeTrue()) + // Delete the specified HASComp resource deleteHASCompCR(hasCompLookupKey) @@ -1042,6 +1125,10 @@ var _ = Describe("Component controller", func() { Context("Create Component with non-exist git url", func() { It("Should fail with error", func() { + beforeCreateTotalReqs := testutil.ToFloat64(metrics.ComponentCreationTotalReqs) + beforeCreateSucceedReqs := testutil.ToFloat64(metrics.ComponentCreationSucceeded) + beforeCreateFailedReqs := testutil.ToFloat64(metrics.ComponentCreationFailed) + ctx := context.Background() beforeImportGitRepoTotalReqs := testutil.ToFloat64(metrics.ImportGitRepoTotalReqs) @@ -1096,6 +1183,10 @@ var _ = Describe("Component controller", func() { Expect(testutil.ToFloat64(metrics.ImportGitRepoSucceeded) > beforeImportGitRepoSucceeded).To(BeTrue()) Expect(testutil.ToFloat64(metrics.ImportGitRepoFailed) == beforeImportGitRepoFailed).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationTotalReqs) > beforeCreateTotalReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationSucceeded) > beforeCreateSucceedReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationFailed) == beforeCreateFailedReqs).To(BeTrue()) + // Delete the specified HASComp resource deleteHASCompCR(hasCompLookupKey) @@ -1106,6 +1197,10 @@ var _ = Describe("Component controller", func() { Context("Create Component with invalid devfile url", func() { It("Should fail with error that devfile couldn't be unmarshalled", func() { + beforeCreateTotalReqs := testutil.ToFloat64(metrics.ComponentCreationTotalReqs) + beforeCreateSucceedReqs := testutil.ToFloat64(metrics.ComponentCreationSucceeded) + beforeCreateFailedReqs := testutil.ToFloat64(metrics.ComponentCreationFailed) + ctx := context.Background() applicationName := HASAppName + "12" @@ -1153,6 +1248,10 @@ var _ = Describe("Component controller", func() { hasAppLookupKey := types.NamespacedName{Name: applicationName, Namespace: HASAppNamespace} + Expect(testutil.ToFloat64(metrics.ComponentCreationTotalReqs) > beforeCreateTotalReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationSucceeded) == beforeCreateSucceedReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationFailed) > beforeCreateFailedReqs).To(BeTrue()) + // Delete the specified HASComp resource deleteHASCompCR(hasCompLookupKey) @@ -1222,6 +1321,10 @@ var _ = Describe("Component controller", func() { Context("Create Component with git secret field set to an invalid secret", func() { It("Should error out due parse error", func() { + beforeCreateTotalReqs := testutil.ToFloat64(metrics.ComponentCreationTotalReqs) + beforeCreateSucceedReqs := testutil.ToFloat64(metrics.ComponentCreationSucceeded) + beforeCreateFailedReqs := testutil.ToFloat64(metrics.ComponentCreationFailed) + // the secret exists but it's not a real one that we can use to access a live repo ctx := context.Background() @@ -1286,6 +1389,10 @@ var _ = Describe("Component controller", func() { Expect(strings.ToLower(createdHasComp.Status.Conditions[len(createdHasComp.Status.Conditions)-1].Message)).Should(ContainSubstring("component create failed: failed to populateandparsedevfile: error getting devfile info from url: failed to retrieve")) hasAppLookupKey := types.NamespacedName{Name: applicationName, Namespace: HASAppNamespace} + Expect(testutil.ToFloat64(metrics.ComponentCreationTotalReqs) > beforeCreateTotalReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationSucceeded) == beforeCreateSucceedReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationFailed) > beforeCreateFailedReqs).To(BeTrue()) + // Delete the specified HASComp resource deleteHASCompCR(hasCompLookupKey) @@ -1296,6 +1403,10 @@ var _ = Describe("Component controller", func() { Context("Create Component with private repo, but no devfile", func() { It("Should error out since no devfile exists", func() { + beforeCreateTotalReqs := testutil.ToFloat64(metrics.ComponentCreationTotalReqs) + beforeCreateSucceedReqs := testutil.ToFloat64(metrics.ComponentCreationSucceeded) + beforeCreateFailedReqs := testutil.ToFloat64(metrics.ComponentCreationFailed) + ctx := context.Background() applicationName := HASAppName + "15" @@ -1359,6 +1470,10 @@ var _ = Describe("Component controller", func() { hasAppLookupKey := types.NamespacedName{Name: applicationName, Namespace: HASAppNamespace} + Expect(testutil.ToFloat64(metrics.ComponentCreationTotalReqs) > beforeCreateTotalReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationSucceeded) == beforeCreateSucceedReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationFailed) > beforeCreateFailedReqs).To(BeTrue()) + // Delete the specified HASComp resource deleteHASCompCR(hasCompLookupKey) @@ -1369,6 +1484,10 @@ var _ = Describe("Component controller", func() { Context("Create Component with with context folder containing no devfile", func() { It("Should error out because a devfile cannot be found", func() { + beforeCreateTotalReqs := testutil.ToFloat64(metrics.ComponentCreationTotalReqs) + beforeCreateSucceedReqs := testutil.ToFloat64(metrics.ComponentCreationSucceeded) + beforeCreateFailedReqs := testutil.ToFloat64(metrics.ComponentCreationFailed) + ctx := context.Background() applicationName := HASAppName + "16" @@ -1415,6 +1534,10 @@ var _ = Describe("Component controller", func() { hasAppLookupKey := types.NamespacedName{Name: applicationName, Namespace: HASAppNamespace} + Expect(testutil.ToFloat64(metrics.ComponentCreationTotalReqs) > beforeCreateTotalReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationSucceeded) > beforeCreateSucceedReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationFailed) == beforeCreateFailedReqs).To(BeTrue()) + // Delete the specified HASComp resource deleteHASCompCR(hasCompLookupKey) @@ -1517,6 +1640,10 @@ var _ = Describe("Component controller", func() { Context("Create Component with Dockerfile URL set", func() { It("Should create successfully and update the Application", func() { + beforeCreateTotalReqs := testutil.ToFloat64(metrics.ComponentCreationTotalReqs) + beforeCreateSucceedReqs := testutil.ToFloat64(metrics.ComponentCreationSucceeded) + beforeCreateFailedReqs := testutil.ToFloat64(metrics.ComponentCreationFailed) + ctx := context.Background() applicationName := HASAppName + "18" @@ -1620,6 +1747,10 @@ var _ = Describe("Component controller", func() { Expect(nameMatched).Should(Equal(true)) Expect(repoLinkMatched).Should(Equal(true)) + Expect(testutil.ToFloat64(metrics.ComponentCreationTotalReqs) > beforeCreateTotalReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationSucceeded) > beforeCreateSucceedReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationFailed) == beforeCreateFailedReqs).To(BeTrue()) + // Delete the specified HASComp resource deleteHASCompCR(hasCompLookupKey) @@ -1630,6 +1761,10 @@ var _ = Describe("Component controller", func() { Context("Create Component with setGitOpsGeneration to true", func() { It("Should create successfully and not create the GitOps resources, and generate the resources once set.", func() { + beforeCreateTotalReqs := testutil.ToFloat64(metrics.ComponentCreationTotalReqs) + beforeCreateSucceedReqs := testutil.ToFloat64(metrics.ComponentCreationSucceeded) + beforeCreateFailedReqs := testutil.ToFloat64(metrics.ComponentCreationFailed) + ctx := context.Background() applicationName := HASAppName + "19" @@ -1675,6 +1810,10 @@ var _ = Describe("Component controller", func() { // Make sure the devfile model was properly set in Component Expect(createdComp.Status.Devfile).Should(Not(Equal(""))) + Expect(testutil.ToFloat64(metrics.ComponentCreationTotalReqs) > beforeCreateTotalReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationSucceeded) > beforeCreateSucceedReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationFailed) == beforeCreateFailedReqs).To(BeTrue()) + // Now change skipGitOpsResourceGeneration to true and validate that the GitOps Resources are generated successfully (by validating the GitOpsResourcesGenerated status condition) createdComp.Spec.SkipGitOpsResourceGeneration = false Expect(k8sClient.Update(ctx, createdComp)).Should(Succeed()) @@ -1702,6 +1841,10 @@ var _ = Describe("Component controller", func() { Context("Create Component with Dockerfile URL set for repo with devfile URL", func() { It("Should create successfully and override local Dockerfile URL references in the Devfile", func() { + beforeCreateTotalReqs := testutil.ToFloat64(metrics.ComponentCreationTotalReqs) + beforeCreateSucceedReqs := testutil.ToFloat64(metrics.ComponentCreationSucceeded) + beforeCreateFailedReqs := testutil.ToFloat64(metrics.ComponentCreationFailed) + ctx := context.Background() applicationName := HASAppName + "20" @@ -1802,6 +1945,10 @@ var _ = Describe("Component controller", func() { Expect(nameMatched).Should(Equal(true)) Expect(repoLinkMatched).Should(Equal(true)) + Expect(testutil.ToFloat64(metrics.ComponentCreationTotalReqs) > beforeCreateTotalReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationSucceeded) > beforeCreateSucceedReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationFailed) == beforeCreateFailedReqs).To(BeTrue()) + // Delete the specified HASComp resource deleteHASCompCR(hasCompLookupKey) @@ -1813,6 +1960,10 @@ var _ = Describe("Component controller", func() { // Cannot test combined failure and recovery scenario since mock test uses the component name which can't be changed Context("Component with empty GitOps repository", func() { It("Should error out", func() { + beforeCreateTotalReqs := testutil.ToFloat64(metrics.ComponentCreationTotalReqs) + beforeCreateSucceedReqs := testutil.ToFloat64(metrics.ComponentCreationSucceeded) + beforeCreateFailedReqs := testutil.ToFloat64(metrics.ComponentCreationFailed) + ctx := context.Background() applicationName := HASAppName + "21" @@ -1894,6 +2045,10 @@ var _ = Describe("Component controller", func() { // Make sure the devfile model was properly set in Component Expect(createdHasComp.Status.Devfile).Should(Not(Equal(""))) + Expect(testutil.ToFloat64(metrics.ComponentCreationTotalReqs) > beforeCreateTotalReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationSucceeded) == beforeCreateSucceedReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationFailed) > beforeCreateFailedReqs).To(BeTrue()) + // Update the devfile to empty to see if errors out createdHasComp.Status.Devfile = "" Expect(k8sClient.Status().Update(ctx, createdHasComp)).Should(Succeed()) @@ -1924,6 +2079,10 @@ var _ = Describe("Component controller", func() { }) Context("Component with valid GitOps repository", func() { It("Should successfully update CR conditions and status", func() { + beforeCreateTotalReqs := testutil.ToFloat64(metrics.ComponentCreationTotalReqs) + beforeCreateSucceedReqs := testutil.ToFloat64(metrics.ComponentCreationSucceeded) + beforeCreateFailedReqs := testutil.ToFloat64(metrics.ComponentCreationFailed) + ctx := context.Background() applicationName := HASAppName + "22" @@ -2010,6 +2169,10 @@ var _ = Describe("Component controller", func() { // Make sure the devfile model was properly set in Component Expect(createdHasComp.Status.Devfile).Should(Not(Equal(""))) + Expect(testutil.ToFloat64(metrics.ComponentCreationTotalReqs) > beforeCreateTotalReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationSucceeded) > beforeCreateSucceedReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationFailed) == beforeCreateFailedReqs).To(BeTrue()) + // Delete the specified HASComp resource deleteHASCompCR(hasCompLookupKey) @@ -2019,6 +2182,10 @@ var _ = Describe("Component controller", func() { }) Context("force generate gitops resource", func() { It("Should successfully update CR conditions and status", func() { + beforeCreateTotalReqs := testutil.ToFloat64(metrics.ComponentCreationTotalReqs) + beforeCreateSucceedReqs := testutil.ToFloat64(metrics.ComponentCreationSucceeded) + beforeCreateFailedReqs := testutil.ToFloat64(metrics.ComponentCreationFailed) + ctx := context.Background() applicationName := HASAppName + "23" @@ -2062,6 +2229,10 @@ var _ = Describe("Component controller", func() { Expect(gitopsCondition.Type).To(Equal("GitOpsResourcesGenerated")) Expect(gitopsCondition.Status).To(Equal(metav1.ConditionTrue)) + Expect(testutil.ToFloat64(metrics.ComponentCreationTotalReqs) > beforeCreateTotalReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationSucceeded) > beforeCreateSucceedReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationFailed) == beforeCreateFailedReqs).To(BeTrue()) + // set the annotation and update a spec to force enter the reconcile setForceGenerateGitopsAnnotation(createdHasComp, "true") createdHasComp.Spec.TargetPort = 1111 @@ -2096,6 +2267,10 @@ var _ = Describe("Component controller", func() { Context("Create Private Component with basic field set", func() { It("Should create successfully and update the Application", func() { + beforeCreateTotalReqs := testutil.ToFloat64(metrics.ComponentCreationTotalReqs) + beforeCreateSucceedReqs := testutil.ToFloat64(metrics.ComponentCreationSucceeded) + beforeCreateFailedReqs := testutil.ToFloat64(metrics.ComponentCreationFailed) + ctx := context.Background() applicationName := HASAppName + "24" @@ -2209,6 +2384,10 @@ var _ = Describe("Component controller", func() { Expect(nameMatched).Should(Equal(true)) Expect(repoLinkMatched).Should(Equal(true)) + Expect(testutil.ToFloat64(metrics.ComponentCreationTotalReqs) > beforeCreateTotalReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationSucceeded) > beforeCreateSucceedReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationFailed) == beforeCreateFailedReqs).To(BeTrue()) + // Delete the specified HASComp resource deleteHASCompCR(hasCompLookupKey) @@ -2219,6 +2398,10 @@ var _ = Describe("Component controller", func() { Context("Create private and public Components for the same Application with basic field set", func() { It("Should create SPI FCR resource and associate it with only the private Component", func() { + beforeCreateTotalReqs := testutil.ToFloat64(metrics.ComponentCreationTotalReqs) + beforeCreateSucceedReqs := testutil.ToFloat64(metrics.ComponentCreationSucceeded) + beforeCreateFailedReqs := testutil.ToFloat64(metrics.ComponentCreationFailed) + ctx := context.Background() applicationName := HASAppName + "25" @@ -2395,6 +2578,10 @@ var _ = Describe("Component controller", func() { Expect(publicNameMatched).Should(Equal(true)) Expect(publicRepoLinkMatched).Should(Equal(true)) + Expect(testutil.ToFloat64(metrics.ComponentCreationTotalReqs) == beforeCreateTotalReqs+2).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationSucceeded) == beforeCreateSucceedReqs+2).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationFailed) == beforeCreateFailedReqs).To(BeTrue()) + // Delete the specified public HASComp resource deleteHASCompCR(hasCompPublicLookupKey) @@ -2429,6 +2616,10 @@ var _ = Describe("Component controller", func() { Context("Create Private Component with basic field set and a private parent uri", func() { It("Should create successfully and update the Application", func() { + beforeCreateTotalReqs := testutil.ToFloat64(metrics.ComponentCreationTotalReqs) + beforeCreateSucceedReqs := testutil.ToFloat64(metrics.ComponentCreationSucceeded) + beforeCreateFailedReqs := testutil.ToFloat64(metrics.ComponentCreationFailed) + ctx := context.Background() applicationName := HASAppName + "26" @@ -2546,6 +2737,10 @@ var _ = Describe("Component controller", func() { Expect(nameMatched).Should(Equal(true)) Expect(repoLinkMatched).Should(Equal(true)) + Expect(testutil.ToFloat64(metrics.ComponentCreationTotalReqs) > beforeCreateTotalReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationSucceeded) > beforeCreateSucceedReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationFailed) == beforeCreateFailedReqs).To(BeTrue()) + // Update Component createdHasComp.Spec.TargetPort = updatedPort @@ -2582,6 +2777,9 @@ var _ = Describe("Component controller", func() { Context("Create private Component for an Application with basic field set", func() { It("Should create SPI FCR resource and persist it even though the associated private Component is in an error state", func() { + beforeCreateTotalReqs := testutil.ToFloat64(metrics.ComponentCreationTotalReqs) + beforeCreateSucceedReqs := testutil.ToFloat64(metrics.ComponentCreationSucceeded) + beforeCreateFailedReqs := testutil.ToFloat64(metrics.ComponentCreationFailed) ctx := context.Background() applicationName := HASAppName + "27" @@ -2642,6 +2840,10 @@ var _ = Describe("Component controller", func() { Expect(createdHasPrivateComp.Status.Conditions[len(createdHasPrivateComp.Status.Conditions)-1].Reason).Should(Equal("Error")) Expect(strings.ToLower(createdHasPrivateComp.Status.Conditions[len(createdHasPrivateComp.Status.Conditions)-1].Message)).Should(ContainSubstring("error getting devfile")) + Expect(testutil.ToFloat64(metrics.ComponentCreationTotalReqs) > beforeCreateTotalReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationSucceeded) == beforeCreateSucceedReqs).To(BeTrue()) + Expect(testutil.ToFloat64(metrics.ComponentCreationFailed) > beforeCreateFailedReqs).To(BeTrue()) + hasAppLookupKey := types.NamespacedName{Name: applicationName, Namespace: HASAppNamespace} createdHasApp := &appstudiov1alpha1.Application{} Eventually(func() bool { diff --git a/controllers/component_controller_unit_test.go b/controllers/component_controller_unit_test.go index d7264056..f09f67c6 100644 --- a/controllers/component_controller_unit_test.go +++ b/controllers/component_controller_unit_test.go @@ -565,3 +565,87 @@ func TestGenerateGitops(t *testing.T) { } } + +func TestCheckForCreateReconcile(t *testing.T) { + + tests := []struct { + name string + component appstudiov1alpha1.Component + wantCreateReconcile bool + }{ + { + name: "Component with Create Condition", + component: appstudiov1alpha1.Component{ + ObjectMeta: metav1.ObjectMeta{ + Name: "comp1", + }, + Spec: appstudiov1alpha1.ComponentSpec{ + ComponentName: "comp1", + }, + Status: appstudiov1alpha1.ComponentStatus{ + Conditions: []metav1.Condition{ + { + Type: "Created", + Status: metav1.ConditionTrue, + Reason: "OK", + Message: "Component has been successfully created", + }, + }, + }, + }, + wantCreateReconcile: true, + }, + { + name: "Component with no Conditions", + component: appstudiov1alpha1.Component{ + ObjectMeta: metav1.ObjectMeta{ + Name: "comp1", + }, + Spec: appstudiov1alpha1.ComponentSpec{ + ComponentName: "comp1", + }, + Status: appstudiov1alpha1.ComponentStatus{}, + }, + wantCreateReconcile: true, + }, + { + name: "Component with Create and Update Conditions", + component: appstudiov1alpha1.Component{ + ObjectMeta: metav1.ObjectMeta{ + Name: "comp1", + }, + Spec: appstudiov1alpha1.ComponentSpec{ + ComponentName: "comp1", + }, + Status: appstudiov1alpha1.ComponentStatus{ + Conditions: []metav1.Condition{ + { + Type: "Created", + Status: metav1.ConditionTrue, + Reason: "OK", + Message: "Component has been successfully created", + }, + { + Type: "Updated", + Status: metav1.ConditionFalse, + Reason: "Error", + Message: "Component updated failed", + }, + }, + }, + }, + wantCreateReconcile: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := checkForCreateReconcile(tt.component) + + if tt.wantCreateReconcile != got { + t.Errorf("TestCheckForCreateReconcile error: expected %v, got %v", tt.wantCreateReconcile, got) + } + }) + } + +} diff --git a/controllers/errors.go b/controllers/errors.go index ab24449d..b1018d38 100644 --- a/controllers/errors.go +++ b/controllers/errors.go @@ -37,3 +37,11 @@ type GitOpsCommitIdError struct { func (e *GitOpsCommitIdError) Error() string { return util.SanitizeErrorMessage(fmt.Errorf("unable to retrieve gitops repository commit id due to error: %v", e.err)).Error() } + +type NotSupported struct { + err error +} + +func (e *NotSupported) Error() string { + return util.SanitizeErrorMessage(fmt.Errorf("not supported error: %v", e.err)).Error() +} diff --git a/controllers/update.go b/controllers/update.go index 9e95a21b..629e70fc 100644 --- a/controllers/update.go +++ b/controllers/update.go @@ -81,7 +81,7 @@ func (r *ComponentReconciler) updateComponentDevfileModel(req ctrl.Request, hasC currentReplica = int(kubernetesComponent.Attributes.GetNumber(devfile.ReplicaKey, &err)) if err != nil { if _, ok := err.(*attributes.KeyNotFoundError); !ok { - return err + return &devfile.DevfileAttributeParse{Key: devfile.ReplicaKey, Err: err} } else { keyFound = false currentReplica = 1 //if an error is raised, it'll set currentReplica to 0 so we need to reset back to the default @@ -130,7 +130,7 @@ func (r *ComponentReconciler) updateComponentDevfileModel(req ctrl.Request, hasC if err != nil { if _, ok := err.(*attributes.KeyNotFoundError); !ok { - return err + return &devfile.DevfileAttributeParse{Key: devfile.ReplicaKey, Err: err} } else { log.Info(fmt.Sprintf("deleting %s attribute with value %v", devfile.ReplicaKey, num)) delete(kubernetesComponent.Attributes, devfile.ReplicaKey) @@ -155,7 +155,7 @@ func (r *ComponentReconciler) updateComponentDevfileModel(req ctrl.Request, hasC currentPort := int(kubernetesComponent.Attributes.GetNumber(devfile.ContainerImagePortKey, &err)) if err != nil { if _, ok := err.(*attributes.KeyNotFoundError); !ok { - return err + return &devfile.DevfileAttributeParse{Key: devfile.ContainerImagePortKey, Err: err} } } if currentPort != component.Spec.TargetPort { @@ -176,12 +176,12 @@ func (r *ComponentReconciler) updateComponentDevfileModel(req ctrl.Request, hasC err = kubernetesComponent.Attributes.GetInto(devfile.ContainerENVKey, ¤tENV) if err != nil { if _, ok := err.(*attributes.KeyNotFoundError); !ok { - return err + return &devfile.DevfileAttributeParse{Key: devfile.ContainerENVKey, Err: err} } } for _, env := range component.Spec.Env { if env.ValueFrom != nil { - return fmt.Errorf("env.ValueFrom is not supported at the moment, use env.value") + return &NotSupported{err: fmt.Errorf("env.ValueFrom is not supported at the moment, use env.value")} } name := env.Name @@ -204,7 +204,7 @@ func (r *ComponentReconciler) updateComponentDevfileModel(req ctrl.Request, hasC var err error kubernetesComponent.Attributes = kubernetesComponent.Attributes.FromMap(map[string]interface{}{devfile.ContainerENVKey: currentENV}, &err) if err != nil { - return err + return &devfile.DevfileAttributeParse{Key: devfile.ContainerENVKey, Err: err} } compUpdateRequired = true } diff --git a/pkg/devfile/devfile.go b/pkg/devfile/devfile.go index 40642599..6e766ef8 100644 --- a/pkg/devfile/devfile.go +++ b/pkg/devfile/devfile.go @@ -61,7 +61,7 @@ func GetResourceFromDevfile(log logr.Logger, devfileData data.DevfileData, deplo matchLabels := getMatchLabel(compName) if len(kubernetesComponents) == 0 { - return parser.KubernetesResources{}, fmt.Errorf("the devfile has no kubernetes components defined, missing outerloop definition") + return parser.KubernetesResources{}, &MissingOuterloop{} } else if len(kubernetesComponents) == 1 && len(deployAssociatedComponents) == 0 { // only one kubernetes components defined, but no deploy cmd associated deployAssociatedComponents[kubernetesComponents[0].Name] = "place-holder" @@ -113,7 +113,7 @@ func GetResourceFromDevfile(log logr.Logger, devfileData data.DevfileData, deplo currentPort := int(component.Attributes.GetNumber(ContainerImagePortKey, &err)) if err != nil { if _, ok := err.(*attributes.KeyNotFoundError); !ok { - return parser.KubernetesResources{}, err + return parser.KubernetesResources{}, &DevfileAttributeParse{Key: ContainerImagePortKey, Err: err} } } @@ -122,7 +122,7 @@ func GetResourceFromDevfile(log logr.Logger, devfileData data.DevfileData, deplo err = component.Attributes.GetInto(ContainerENVKey, ¤tENV) if err != nil { if _, ok := err.(*attributes.KeyNotFoundError); !ok { - return parser.KubernetesResources{}, err + return parser.KubernetesResources{}, &DevfileAttributeParse{Key: ContainerENVKey, Err: err} } } @@ -132,7 +132,7 @@ func GetResourceFromDevfile(log logr.Logger, devfileData data.DevfileData, deplo currentReplica := int32(component.Attributes.GetNumber(ReplicaKey, &replicaErr)) if replicaErr != nil { if _, ok := replicaErr.(*attributes.KeyNotFoundError); !ok { - return parser.KubernetesResources{}, err + return parser.KubernetesResources{}, &DevfileAttributeParse{Key: ReplicaKey, Err: err} } else { // check the deployment to see what value is set replica := resources.Deployments[0].Spec.Replicas @@ -182,8 +182,6 @@ func GetResourceFromDevfile(log logr.Logger, devfileData data.DevfileData, deplo resources.Deployments[0].Spec.Template.ObjectMeta.Labels = matchLabels } - //resources.Deployments[0].Spec.Replicas = ¤tReplica - if len(resources.Deployments[0].Spec.Template.Spec.Containers) > 0 { if image != "" { resources.Deployments[0].Spec.Template.Spec.Containers[0].Image = image @@ -226,21 +224,21 @@ func GetResourceFromDevfile(log logr.Logger, devfileData data.DevfileData, deplo cpuLimit := component.Attributes.GetString(CpuLimitKey, &err) if err != nil { if _, ok := err.(*attributes.KeyNotFoundError); !ok { - return parser.KubernetesResources{}, err + return parser.KubernetesResources{}, &DevfileAttributeParse{Key: CpuLimitKey, Err: err} } } memoryLimit := component.Attributes.GetString(MemoryLimitKey, &err) if err != nil { if _, ok := err.(*attributes.KeyNotFoundError); !ok { - return parser.KubernetesResources{}, err + return parser.KubernetesResources{}, &DevfileAttributeParse{Key: MemoryLimitKey, Err: err} } } storageLimit := component.Attributes.GetString(StorageLimitKey, &err) if err != nil { if _, ok := err.(*attributes.KeyNotFoundError); !ok { - return parser.KubernetesResources{}, err + return parser.KubernetesResources{}, &DevfileAttributeParse{Key: StorageLimitKey, Err: err} } } @@ -252,7 +250,7 @@ func GetResourceFromDevfile(log logr.Logger, devfileData data.DevfileData, deplo if cpuLimit != "" && cpuLimit != "0" { cpuLimitQuantity, err := resource.ParseQuantity(cpuLimit) if err != nil { - return parser.KubernetesResources{}, err + return parser.KubernetesResources{}, &DevfileAttributeParse{Key: CpuLimitKey, Err: err} } containerLimits[corev1.ResourceCPU] = cpuLimitQuantity } @@ -260,7 +258,7 @@ func GetResourceFromDevfile(log logr.Logger, devfileData data.DevfileData, deplo if memoryLimit != "" && memoryLimit != "0" { memoryLimitQuantity, err := resource.ParseQuantity(memoryLimit) if err != nil { - return parser.KubernetesResources{}, err + return parser.KubernetesResources{}, &DevfileAttributeParse{Key: MemoryLimitKey, Err: err} } containerLimits[corev1.ResourceMemory] = memoryLimitQuantity } @@ -268,7 +266,7 @@ func GetResourceFromDevfile(log logr.Logger, devfileData data.DevfileData, deplo if storageLimit != "" && storageLimit != "0" { storageLimitQuantity, err := resource.ParseQuantity(storageLimit) if err != nil { - return parser.KubernetesResources{}, err + return parser.KubernetesResources{}, &DevfileAttributeParse{Key: StorageLimitKey, Err: err} } containerLimits[corev1.ResourceStorage] = storageLimitQuantity } @@ -279,21 +277,21 @@ func GetResourceFromDevfile(log logr.Logger, devfileData data.DevfileData, deplo cpuRequest := component.Attributes.GetString(CpuRequestKey, &err) if err != nil { if _, ok := err.(*attributes.KeyNotFoundError); !ok { - return parser.KubernetesResources{}, err + return parser.KubernetesResources{}, &DevfileAttributeParse{Key: CpuRequestKey, Err: err} } } memoryRequest := component.Attributes.GetString(MemoryRequestKey, &err) if err != nil { if _, ok := err.(*attributes.KeyNotFoundError); !ok { - return parser.KubernetesResources{}, err + return parser.KubernetesResources{}, &DevfileAttributeParse{Key: MemoryRequestKey, Err: err} } } storageRequest := component.Attributes.GetString(StorageRequestKey, &err) if err != nil { if _, ok := err.(*attributes.KeyNotFoundError); !ok { - return parser.KubernetesResources{}, err + return parser.KubernetesResources{}, &DevfileAttributeParse{Key: StorageRequestKey, Err: err} } } @@ -305,7 +303,7 @@ func GetResourceFromDevfile(log logr.Logger, devfileData data.DevfileData, deplo if cpuRequest != "" && cpuRequest != "0" { cpuRequestQuantity, err := resource.ParseQuantity(cpuRequest) if err != nil { - return parser.KubernetesResources{}, err + return parser.KubernetesResources{}, &DevfileAttributeParse{Key: CpuRequestKey, Err: err} } containerRequests[corev1.ResourceCPU] = cpuRequestQuantity } @@ -313,7 +311,7 @@ func GetResourceFromDevfile(log logr.Logger, devfileData data.DevfileData, deplo if memoryRequest != "" && memoryRequest != "0" { memoryRequestQuantity, err := resource.ParseQuantity(memoryRequest) if err != nil { - return parser.KubernetesResources{}, err + return parser.KubernetesResources{}, &DevfileAttributeParse{Key: MemoryRequestKey, Err: err} } containerRequests[corev1.ResourceMemory] = memoryRequestQuantity } @@ -321,7 +319,7 @@ func GetResourceFromDevfile(log logr.Logger, devfileData data.DevfileData, deplo if storageRequest != "" && storageRequest != "0" { storageRequestQuantity, err := resource.ParseQuantity(storageRequest) if err != nil { - return parser.KubernetesResources{}, err + return parser.KubernetesResources{}, &DevfileAttributeParse{Key: StorageRequestKey, Err: err} } containerRequests[corev1.ResourceStorage] = storageRequestQuantity } @@ -411,7 +409,7 @@ func GetResourceFromDevfile(log logr.Logger, devfileData data.DevfileData, deplo route := component.Attributes.GetString(RouteKey, &err) if err != nil { if _, ok := err.(*attributes.KeyNotFoundError); !ok { - return parser.KubernetesResources{}, err + return parser.KubernetesResources{}, &DevfileAttributeParse{Key: RouteKey, Err: err} } } diff --git a/pkg/devfile/errors.go b/pkg/devfile/errors.go index 7f2cfa02..28ec2f94 100644 --- a/pkg/devfile/errors.go +++ b/pkg/devfile/errors.go @@ -30,3 +30,23 @@ func (e *NoFileFound) Error() string { } return errMsg } + +// MissingOuterloop returns an error if no Kubernetes Component was found in a Devfile +type MissingOuterloop struct { +} + +func (e *MissingOuterloop) Error() string { + return "the devfile has no kubernetes components defined, missing outerloop definition" +} + +// DevfileAttributeParse returns an error if was an issue parsing the attribute key +type DevfileAttributeParse struct { + Key string + Err error +} + +func (e *DevfileAttributeParse) Error() string { + errMsg := fmt.Sprintf("error parsing key %s: %v", e.Key, e.Err) + + return errMsg +} diff --git a/pkg/github/github.go b/pkg/github/github.go index 1193d143..9ffc1a1f 100644 --- a/pkg/github/github.go +++ b/pkg/github/github.go @@ -143,11 +143,14 @@ func (g *GitHubClient) GetDefaultBranchFromURL(repoURL string, ctx context.Conte func (g *GitHubClient) GetBranchFromURL(repoURL string, ctx context.Context, branchName string) (*github.Branch, error) { repoName, orgName, err := GetRepoAndOrgFromURL(repoURL) if err != nil { + // User error - so increment the "success" metric - since we're tracking only system errors + metrics.ComponentCreationSucceeded.Inc() return nil, err } branch, _, err := g.Client.Repositories.GetBranch(ctx, orgName, repoName, branchName, false) if err != nil || branch == nil { + metrics.ComponentCreationFailed.Inc() return nil, fmt.Errorf("failed to get branch %s from repo %s under %s, error: %v", branchName, repoName, orgName, err) } diff --git a/pkg/github/github_test.go b/pkg/github/github_test.go index e725e3e6..162026aa 100644 --- a/pkg/github/github_test.go +++ b/pkg/github/github_test.go @@ -504,6 +504,9 @@ func TestGetBranchFromURL(t *testing.T) { Client: GetMockedClient(), } + beforeCreateSuccess := testutil.ToFloat64(metrics.ComponentCreationSucceeded) + beforeCreateFailed := testutil.ToFloat64(metrics.ComponentCreationFailed) + t.Run(tt.name, func(t *testing.T) { branch, err := mockedClient.GetBranchFromURL(tt.repoURL, context.Background(), tt.branchName) @@ -513,6 +516,18 @@ func TestGetBranchFromURL(t *testing.T) { if !tt.wantErr && *branch.Name != tt.branchName { t.Errorf("TestGetBranchFromURL() error: expected %v got %v", tt.branchName, *branch.Name) } + + if tt.wantErr { + if tt.name == "Unparseable URL" { + if testutil.ToFloat64(metrics.ComponentCreationSucceeded) <= beforeCreateSuccess { + t.Errorf("TestGetBranchFromURL() expected metric 'ComponentCreationSucceeded' to be incremented, beforeCreateSuccess %v and testutil.ToFloat64(metrics.ComponentCreationSucceeded) %v", beforeCreateSuccess, testutil.ToFloat64(metrics.ComponentCreationSucceeded)) + } + } else if tt.name == "Simple repo name" { + if testutil.ToFloat64(metrics.ComponentCreationFailed) <= beforeCreateFailed { + t.Errorf("TestGetBranchFromURL() expected metric 'ComponentCreationFailed' to be incremented, beforeCreateFailed %v and testutil.ToFloat64(metrics.ComponentCreationFailed) %v", beforeCreateFailed, testutil.ToFloat64(metrics.ComponentCreationFailed)) + } + } + } }) } } diff --git a/pkg/metrics/metrics.go b/pkg/metrics/metrics.go index c715dc09..625dbbe6 100644 --- a/pkg/metrics/metrics.go +++ b/pkg/metrics/metrics.go @@ -117,6 +117,26 @@ var ( }, ) + ComponentCreationTotalReqs = prometheus.NewCounter( + prometheus.CounterOpts{ + Name: "has_component_creation_total", + Help: "Number of component creation requests processed", + }, + ) + ComponentCreationFailed = prometheus.NewCounter( + prometheus.CounterOpts{ + Name: "has_component_failed_creation_total", + Help: "Number of failed component creation requests", + }, + ) + + ComponentCreationSucceeded = prometheus.NewCounter( + prometheus.CounterOpts{ + Name: "has_component_successful_creation_total", + Help: "Number of successful component creation requests", + }, + ) + ComponentDeletionTotalReqs = prometheus.NewCounter( prometheus.CounterOpts{ Name: "has_component_deletion_total", @@ -160,9 +180,12 @@ var ( func init() { // Register custom metrics with the global prometheus registry - metrics.Registry.MustRegister(GitOpsRepoCreationTotalReqs, GitOpsRepoCreationFailed, GitOpsRepoCreationSucceeded, ControllerGitRequest, SecondaryRateLimitCounter, - PrimaryRateLimitCounter, TokenPoolGauge, ApplicationDeletionTotalReqs, ApplicationDeletionSucceeded, ApplicationDeletionFailed, - ApplicationCreationSucceeded, ApplicationCreationFailed, ApplicationCreationTotalReqs, ComponentDeletionTotalReqs, ComponentDeletionSucceeded, ComponentDeletionFailed, + metrics.Registry.MustRegister(GitOpsRepoCreationTotalReqs, GitOpsRepoCreationFailed, GitOpsRepoCreationSucceeded, ControllerGitRequest, + SecondaryRateLimitCounter, PrimaryRateLimitCounter, TokenPoolGauge, + ApplicationDeletionTotalReqs, ApplicationDeletionSucceeded, ApplicationDeletionFailed, + ApplicationCreationSucceeded, ApplicationCreationFailed, ApplicationCreationTotalReqs, + ComponentCreationTotalReqs, ComponentCreationSucceeded, ComponentCreationFailed, + ComponentDeletionTotalReqs, ComponentDeletionSucceeded, ComponentDeletionFailed, ImportGitRepoTotalReqs, ImportGitRepoFailed, ImportGitRepoSucceeded) }