Skip to content

Conversation

kumarajeet
Copy link
Contributor

@kumarajeet kumarajeet commented Feb 2, 2021

Overview
This PR is for a story as part of AppFramework Operator-Setup. This story provides configuration section to pass the App Framework related parameters.

It is applicable to following CRs:
Standalone, ClusterMaster, SearchHeadCluster and LicenseMaster

By default, this feature is disabled unless we explicitly enable it by adding following under "applications" section

featureEnabled: TRUE

Currently unit tests are present for the configuration values for :

   appFrameworkRef:
     type: s3
     s3Endpoint: "http://10.244.0.6:9000"
     s3Bucket: "team1cluster"
     s3SecretRef: mysecret
     s3PollInterval: 60 


func TestValidateApplicationFrameworkSpec(t *testing.T) {
var err error
// Valid app framework config with proper inputs
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a Positive test case too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, let me check.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Positive check was also added.

Copy link
Collaborator

@akondur akondur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Couple of minor comments.

1) Log only if apps framework feature flag is true
2) Use scoped logging
description: App Package Remote Store Endpoint
type: string
s3PollInterval:
description: App Package Remote Store Polling interval
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably add the units and default value here so users know what they are configuring.

Also I noticed that it looks like the other code is written with seconds in mind, from our earlier discussion did we want to go with minutes instead?

@kumarajeet kumarajeet closed this Feb 12, 2021
@kumarajeet kumarajeet deleted the CSPL-763-AppFramework-Add-new-params branch February 12, 2021 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants