-
Notifications
You must be signed in to change notification settings - Fork 125
CSPL-3549 Splunk Operator Enhancement – Ingestion and Indexing Separation #1550
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
ca230a9 to
eb21049
Compare
eb21049 to
2cca0d7
Compare
Pull Request Test Coverage Report for Build 17610018166Details
💛 - Coveralls |
d3779b0 to
44349fa
Compare
3b2cf1c to
5cad7f9
Compare
8e32bd1 to
4df319e
Compare
| Replicas int32 `json:"replicas"` | ||
|
|
||
| // Splunk Enterprise app repository that specifies remote app location and scope for Splunk app management | ||
| AppFrameworkConfig AppFrameworkSpec `json:"appRepo,omitempty"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't have to happen right now, but when we do more productization, we should update the AppFramework and CustomResources docs to include this CR.
| - op: add | ||
| path: "/spec/versions/-" | ||
| value: | ||
| name: v1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this be supported for v1 and v2? I don't see these files for clustermanager and licensemanager for example, which were later CRs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It got generated by make command as far as I remember. Will revisit it later.
| # without permissions to modify them. It is ideal for monitoring purposes and limited-access viewing. | ||
|
|
||
| apiVersion: rbac.authorization.k8s.io/v1 | ||
| kind: ClusterRole |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will probably need to add this to the helm chart as well: https://github.com/splunk/splunk-operator/tree/main/helm-chart/splunk-operator/templates/rbac
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
| - clustermanagers | ||
| - clustermasters | ||
| - indexerclusters | ||
| - ingestorclusters |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will probably need to add this to the helm chart as well: https://github.com/splunk/splunk-operator/tree/main/helm-chart/splunk-operator/templates/rbac
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
f2d467a to
713eac7
Compare
713eac7 to
1c77634
Compare
b12dbb4 to
68e8c7c
Compare
68e8c7c to
e62f381
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
few changes are requested
| utilruntime.Must(clientgoscheme.AddToScheme(scheme)) | ||
| utilruntime.Must(enterpriseApi.AddToScheme(scheme)) | ||
| utilruntime.Must(enterpriseApiV3.AddToScheme(scheme)) | ||
| utilruntime.Must(enterprisev4.AddToScheme(scheme)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we already have utilruntime.Must(enterpriseApi.AddToScheme(scheme)) which points to v4
| # without permissions to modify them. It is ideal for monitoring purposes and limited-access viewing. | ||
|
|
||
| apiVersion: rbac.authorization.k8s.io/v1 | ||
| kind: ClusterRole |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
| - clustermanagers | ||
| - clustermasters | ||
| - indexerclusters | ||
| - ingestorclusters |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
|
|
||
| request, err := http.NewRequest("POST", endpoint, strings.NewReader(body)) | ||
| if err != nil { | ||
| return err |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
log error here, also we need to make sure to add events which shows stages of CR status.
| if cr.Spec.PullBus.Type != "" { | ||
| err = mgr.handlePullBusOrPipelineConfigChange(ctx, cr, client) | ||
| if err != nil { | ||
| scopedLog.Error(err, "Failed to update conf file for PullBus/Pipeline config change after pod creation") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add an k8s event here ( or at least one failure) and stages of Ingestor state
| var newSplunkClientForPullBusPipeline = splclient.NewSplunkClient | ||
|
|
||
| // Checks if only PullBus or Pipeline config changed, and updates the conf file if so | ||
| func (mgr *indexerClusterPodManager) handlePullBusOrPipelineConfigChange(ctx context.Context, newCR *enterpriseApi.IndexerCluster, k8s client.Client) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: Not related to this feature , but on longer run we need to find a way to bring these changes to SOK https://github.com/splunk/splunk-operator/pull/1193/files. it just simplifies how we manage Rest calls
| return updateErr | ||
| } | ||
|
|
||
| func getChangedPullBusAndPipelineFieldsIndexer(newCR *enterpriseApi.IndexerCluster) (pullBusChangedFieldsInputs, pullBusChangedFieldsOutputs, pipelineChangedFields [][]string) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add description to the function
| case SplunkMonitoringConsole: | ||
| role = "splunk_monitor" | ||
| case SplunkIngestor: | ||
| role = "splunk_standalone" // TODO: change this to a new role when we have one (splunk_ingestor) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought @Igor-splunk added role to ansible
| SplunkLicenseMaster: splcommon.LicenseManagerRole, | ||
| SplunkLicenseManager: splcommon.LicenseManagerRole, | ||
| SplunkMonitoringConsole: "splunk_monitor", | ||
| SplunkIngestor: "splunk_standalone", // TODO: change this to a new role when we have one (splunk_ingestor) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here , have we added new role to ansible
Description
This PR implements a separation between ingestion and indexing services within the Splunk Operator for Kubernetes. The goal is to enable the operator to independently manage the ingestion service while maintaining seamless integration with the indexing service.
Key Changes
Testing and Verification
These changed were tested manually and as a part of the further enhancements, automated tests will be implemented.
Related Issues
Jira Epic: https://splunk.atlassian.net/browse/CSPL-3549
Notes
PR Checklist