Skip to content

Commit

Permalink
Add prometheus metrics for Component creation (#431)
Browse files Browse the repository at this point in the history 
Signed-off-by: Maysun J Faisal <[email protected]>
  • Loading branch information
@maysunfaisal
maysunfaisal authored Dec 13, 2023
1 parent 2fc5811 commit 109395e
Show file tree
Hide file tree
Showing 10 changed files with 483 additions and 34 deletions.
108 changes: 102 additions & 6 deletions controllers/component_controller.go
View file

Large diffs are not rendered by default.

202 changes: 202 additions & 0 deletions controllers/component_controller_test.go
View file

Large diffs are not rendered by default.

84 changes: 84 additions & 0 deletions controllers/component_controller_unit_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
}

}
8 changes: 8 additions & 0 deletions controllers/errors.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
12 changes: 6 additions & 6 deletions controllers/update.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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 {
Expand All @@ -176,12 +176,12 @@ func (r *ComponentReconciler) updateComponentDevfileModel(req ctrl.Request, hasC
err = kubernetesComponent.Attributes.GetInto(devfile.ContainerENVKey, &currentENV)
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
Expand All @@ -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
}
Expand Down
36 changes: 17 additions & 19 deletions pkg/devfile/devfile.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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}
}
}

Expand All @@ -122,7 +122,7 @@ func GetResourceFromDevfile(log logr.Logger, devfileData data.DevfileData, deplo
err = component.Attributes.GetInto(ContainerENVKey, &currentENV)
if err != nil {
if _, ok := err.(*attributes.KeyNotFoundError); !ok {
return parser.KubernetesResources{}, err
return parser.KubernetesResources{}, &DevfileAttributeParse{Key: ContainerENVKey, Err: err}
}
}

Expand All @@ -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
Expand Down Expand Up @@ -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 = &currentReplica

if len(resources.Deployments[0].Spec.Template.Spec.Containers) > 0 {
if image != "" {
resources.Deployments[0].Spec.Template.Spec.Containers[0].Image = image
Expand Down Expand Up @@ -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}
}
}

Expand All @@ -252,23 +250,23 @@ 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
}

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
}

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
}
Expand All @@ -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}
}
}

Expand All @@ -305,23 +303,23 @@ 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
}

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
}

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
}
Expand Down Expand Up @@ -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}
}
}

Expand Down
20 changes: 20 additions & 0 deletions pkg/devfile/errors.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
3 changes: 3 additions & 0 deletions pkg/github/github.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
15 changes: 15 additions & 0 deletions pkg/github/github_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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))
}
}
}
})
}
}
Loading

0 comments on commit 109395e

Please sign in to comment.