Skip to content

Conversation

sgontla
Copy link
Contributor

@sgontla sgontla commented Jun 8, 2021

  • Added new configMap to update the latest apps status to the pod locations

@sgontla sgontla requested review from smohan-splunk, gaurav-splunk, jryb and kashok-splunk and removed request for smohan-splunk and gaurav-splunk June 8, 2021 05:08
@sgontla sgontla force-pushed the feature/CSPL-767 branch from 4ba82b6 to 268c54a Compare June 10, 2021 00:21
@sgontla sgontla force-pushed the feature/CSPL-767 branch 4 times, most recently from 5051b47 to eed23fa Compare June 10, 2021 23:46
returnedCM := PrepareConfigMap(configMapName, namespace, dataMap)

if !reflect.DeepEqual(expectedCm, returnedCM) {
t.Errorf("configMap preperation failed")
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Preparation


// ApplyAppListingConfigMap creates the configMap with two entries:
// (1) app-list-local.yaml
// (2) app-list-cluster.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

Update comment to clarify that the config map will be used by ansible to install apps


// Apply the configMap with a fresh token
SplunkOperatorAppConfigMap = prepareSplunkSmartstoreConfigMap(cr.GetName(), cr.GetNamespace(), crKind, mapSplunkConfDetails)
SplunkOperatorAppConfigMap := splctrl.PrepareConfigMap(configMapName, cr.GetNamespace(), mapSplunkConfDetails)
Copy link
Contributor

Choose a reason for hiding this comment

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

Duplicate SplunkOperatorAppConfigMap to be removed


// check if the apps need to be downloaded from remote storage
// ToDo: sgontla: Once after we have the flow, move the following logic into a seperate function(Note:- all errors should not cause a return in that function)
if cr.Spec.CommonSplunkSpec.Mock != true && !reflect.DeepEqual(cr.Status.AppContext.AppFrameworkConfig, cr.Spec.AppFrameworkConfig) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Not introduced as part of this PR, but do we need this Mock check here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we need this clean-up. I Will add a ToDo note for now.

- Added new configMap to update the latest apps status to the pod locations
@sgontla sgontla force-pushed the feature/CSPL-767 branch from eed23fa to 1977e8c Compare June 11, 2021 01:33
@smohan-splunk smohan-splunk merged commit 484e1e9 into feature-appframework Jun 11, 2021
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