feat: Adds Stream Processor update support #3180

jwongmongodb · 2025-03-18T02:15:28Z

Description

This PR adds support to modify a stream processor by calling the existing ModifyStreamProcessor endpoint.

I also added a suite of acceptance tests to cover all the possible state transitions. The main behavior is that a stream processor must be in the CREATED or STOPPED state before it can call the ModifyStreamProcessor endpoint. If the stream processor is currently in a STARTED state, it must be stopped before it can be modified.

Link to any related issue(s):
CLOUDP-301766

Type of change:

Bug fix (non-breaking change which fixes an issue). Please, add the "bug" label to the PR.
New feature (non-breaking change which adds functionality). Please, add the "enhancement" label to the PR. A migration guide must be created or updated if the new feature will go in a major version.
Breaking change (fix or feature that would cause existing functionality to not work as expected). Please, add the "breaking change" label to the PR. A migration guide must be created or updated.
This change requires a documentation update
Documentation fix/enhancement

Required Checklist:

I have signed the MongoDB CLA
I have read the contributing guides
I have checked that this change does not generate any credentials and that they are NOT accidentally logged anywhere.
I have added tests that prove my fix is effective or that my feature works per HashiCorp requirements
I have added any necessary documentation (if appropriate)
I have run make fmt and formatted my code
If changes include deprecations or removals I have added appropriate changelog entries.
If changes include removal or addition of 3rd party GitHub actions, I updated our internal document. Reach out to the APIx Integration slack channel to get access to the internal document.

Further comments

jwongmongodb

Initial draft review to make sure that the logic makes sense before cleaning up.

jwongmongodb · 2025-03-18T02:18:03Z

internal/service/streamprocessor/resource_test.go

-		processorName = "new-processor"
-		instanceName  = acc.RandomName()
-	)
+func TestAccStreamProcessor_StateTransitionsUpdates(t *testing.T) {


I added a whole new test suite since the current config func would be messy to modify for testing all the updates without creating external resources (clusters or kafka). The update that I am check is the pipeline change with tumbling window since this requires no external resources to spin for each of my test case (there are many combinations)

I have three separate test suits for update tests, I remove some of the existing tests here since my new test suits should cover those flows.

The three update suites cover the following:

Valid state to state updates

Valid state to empty state updates, it should use the previous state (CREATE state will have CREATE state after the update with empty state).

Invalid state updates with expected errors.

really nice abstraction for the tests!

jwongmongodb · 2025-03-18T02:22:50Z

internal/service/streamprocessor/resource_test.go

+		`, projectID, instanceName, processorName, pipeline, state)
+}
+
+func checkConfigAttribute(projectID, instanceName, processorName, state, expectedPipelineStr string) resource.TestCheckFunc {


I can rename this checkConfigAttribute func to have the word update in it or I can merge with the existing composeStreamProcessorChecks (would complicate that function if I were to do that so I am leaning with a separate function)

jwongmongodb · 2025-03-18T02:23:51Z

internal/service/streamprocessor/state_transition.go

@@ -43,6 +43,22 @@ func WaitStateTransition(ctx context.Context, requestParams *admin.GetStreamProc
 	return nil, errors.New("did not obtain valid result when waiting for stream processor state transition")
 }

+func ValidateUpdateStateTransition(currentState, plannedState string) (errMsg string, isValidTransition bool) {


I believe the validation should live in the state transition? I also added unit tests for this function that may be overkill, I can see the argument to remove or keep so I defer to the team's opinion

Makes sense to put it here. I like the unit tests for this, it's a nice way to "document" all the possible state transitions

oarbusi

Really good job!

internal/service/streamprocessor/resource.go

internal/service/streamprocessor/model.go

oarbusi · 2025-03-18T09:00:54Z

docs/resources/stream_processor.md

@@ -141,7 +128,7 @@ output "stream_processors_results" {
 ### Optional

 - `options` (Attributes) Optional configuration for the stream processor. (see [below for nested schema](#nestedatt--options))
- `state` (String) The state of the stream processor. Commonly occurring states are 'CREATED', 'STARTED', 'STOPPED' and 'FAILED'. Used to start or stop the Stream Processor. Valid values are `CREATED`, `STARTED` or `STOPPED`. When a Stream Processor is created without specifying the state, it will default to `CREATED` state.
+- `state` (String) The state of the stream processor. Commonly occurring states are 'CREATED', 'STARTED', 'STOPPED' and 'FAILED'. Used to start or stop the Stream Processor. Valid values are `CREATED`, `STARTED` or `STOPPED`. When a Stream Processor is created without specifying the state, it will default to `CREATED` state. When a Stream Processor is updated without specifying the state, it will default to the Previous state. 


I think this detail might not be necessary. The expectation in Terraform is that a non modified attribute will not be changed. Maybe we should mention in a more general level the fact that the stream will be stopped. Maybe something similar to:

**NOTE**: When updating an Atlas Stream Processor, the following behavior applies: 1. If the processor is in a `STARTED` state, it will automatically be stopped before the update is applied 2. The update will be performed while the processor is in `STOPPED` state 3. If the processor was originally in `STARTED` state, it will be restarted after the update

docs team can confirm when they review

what about something like this?

***NOTE***: The following behavior applies to an Atlas Stream Processor that is in a STARTED state: 1. The processor will automatically be stopped before the update is applied. 2. The update will be performed while the processor is in a `STOPPED` state. 3. The processor will be restarted after the update if the config state field is empty or set to `STARTED`.

I don't think this behavior is specific to TF right? Is this something that can be captured in the API spec instead so we can simply add a link here?

The TF behavior may calling multiple endpoints at once (STOP, GET and MODIFY endpoint). Due to the nature of the stream processor, it can only be modified in the CREATED or STOPPED state. I don't think we can have one specific link to a particular endpoint like MODIFY. Perhaps we can add a requirement that the stream processor must be STOPPED before we can modify it.

Currently, the client needs to stop, modify, then start again a processor that is running. The update/modify API does not do it. So in TF we are removing this complexity to have a better user experience

got it, in that case, I like @oarbusi 's suggestion. Although I'd say we should add it as a separate note to call out what happens during an update instead of adding this note to state.
For the state description I think the current one makes sense, maybe rephrase a bit like "When a Stream Processor is updated without specifying the state, it is stopped & then restored to previous state upon update completion."

this is now changed with this commit. Can you also take another look @davidhou17 to see if these documentation changes look good?

oarbusi · 2025-03-18T09:09:30Z

internal/service/streamprocessor/resource_test.go

-		processorName = "new-processor"
-		instanceName  = acc.RandomName()
-	)
+func TestAccStreamProcessor_StateTransitionsUpdates(t *testing.T) {


really nice abstraction for the tests!

oarbusi · 2025-03-18T09:11:59Z

internal/service/streamprocessor/state_transition.go

@@ -43,6 +43,22 @@ func WaitStateTransition(ctx context.Context, requestParams *admin.GetStreamProc
 	return nil, errors.New("did not obtain valid result when waiting for stream processor state transition")
 }

+func ValidateUpdateStateTransition(currentState, plannedState string) (errMsg string, isValidTransition bool) {


Makes sense to put it here. I like the unit tests for this, it's a nice way to "document" all the possible state transitions

github-actions · 2025-03-19T12:11:34Z

APIx bot: a message has been sent to Docs Slack channel

internal/service/streamprocessor/resource_test.go

jasonrydberg

lgtm

maastha · 2025-03-20T11:44:07Z

internal/service/streamprocessor/state_transition_test.go

+			name:           "Invalid transition - CREATED to STOPPED",
+			currentState:   CreatedState,
+			plannedState:   StoppedState,
+			wantErrMsg:     "Stream Processor must be in STARTED state to transition to STOPPED state",


The error message can be a constant that can be used here and in the resource

maastha · 2025-03-20T11:53:09Z

internal/service/streamprocessor/state_transition.go

+		return fmt.Sprintf("Stream Processor must be in %s state to transition to %s state", StartedState, StoppedState), false
+	}
+
+	if plannedState == CreatedState && currentState != CreatedState {


Is there any case where the user can manually set the state to "CREATED"? Looks like it would be something that would always be returned from the API, and the user can only update the state to Start/Stop, is that correct?

The user can also put CREATED as a state when they first create the resource, so leaving it in the config and changing other areas will trigger an update with the CREATED state still in place for the update. Leaving it blanks during the creation part also defaults to the CREATED state.

Thank you for the clarification!

lantoli · 2025-03-20T12:50:11Z

.changelog/3180.txt

@@ -0,0 +1,3 @@
+```release-note:enhancement
+resource/mongodbatlas_stream_processor: Support modifying a stream processor


Suggested change

resource/mongodbatlas_stream_processor: Support modifying a stream processor

resource/mongodbatlas_stream_processor: Support modifying a stream processor

Suggested change

resource/mongodbatlas_stream_processor: Support modifying a stream processor

resource/mongodbatlas_stream_processor: Adds update support

lantoli · 2025-03-20T12:56:40Z

nit: you can use longer branch names as long as prefixed with ticket in case it helps you to know what the branch is about, e.g. CLOUDP-301766_modify_stream_processor

internal/service/streamprocessor/state_transition.go

internal/service/streamprocessor/resource.go

internal/service/streamprocessor/resource_test.go

internal/service/streamprocessor/state_transition.go

EspenAlbert

Thank you for addressing the comments!

AgustinBettati

LGTM, minor comment on the note in docs

AgustinBettati · 2025-03-24T15:13:07Z

docs/data-sources/stream_processor.md

@@ -2,6 +2,11 @@

 `mongodbatlas_stream_processor` describes a stream processor.

+**NOTE**: When updating an Atlas Stream Processor, the following behavior applies:


I believe the intention was to add this in resource docs right? Within the context of the data source docs this is not too relevant.

yes, this was a mistake. I've moved the note to the correct template and regenerated the docs.

hshiozawa · 2025-04-16T23:10:48Z

👍

feat: Supports stream processor modification

303a62a

github-actions bot added the enhancement label Mar 18, 2025

jwongmongodb commented Mar 18, 2025

View reviewed changes

oarbusi reviewed Mar 18, 2025

View reviewed changes

use ModifyStreamProcessorApiParams to call the modify api endpoint

8cff3a4

jwongmongodb marked this pull request as ready for review March 19, 2025 12:11

jwongmongodb requested review from a team as code owners March 19, 2025 12:11

jwongmongodb requested a review from jasonrydberg March 19, 2025 12:12

davidhou17 approved these changes Mar 19, 2025

View reviewed changes

jasonrydberg reviewed Mar 19, 2025

View reviewed changes

internal/service/streamprocessor/resource_test.go Outdated Show resolved Hide resolved

jasonrydberg approved these changes Mar 19, 2025

View reviewed changes

maastha reviewed Mar 20, 2025

View reviewed changes

lantoli reviewed Mar 20, 2025

View reviewed changes

lantoli approved these changes Mar 20, 2025

View reviewed changes

EspenAlbert reviewed Mar 20, 2025

View reviewed changes

internal/service/streamprocessor/state_transition.go Outdated Show resolved Hide resolved

EspenAlbert reviewed Mar 20, 2025

View reviewed changes

internal/service/streamprocessor/resource.go Outdated Show resolved Hide resolved

EspenAlbert reviewed Mar 20, 2025

View reviewed changes

internal/service/streamprocessor/resource.go Outdated Show resolved Hide resolved

EspenAlbert reviewed Mar 20, 2025

View reviewed changes

internal/service/streamprocessor/resource_test.go Outdated Show resolved Hide resolved

EspenAlbert reviewed Mar 20, 2025

View reviewed changes

internal/service/streamprocessor/resource_test.go Outdated Show resolved Hide resolved

jwongmongodb added 4 commits March 20, 2025 10:29

remove json encoding for the streams pipeline in tests

46193ad

use resource.ParallelTest and refactor variable assignments

b88a6b3

tweak errors and use as constants

501260c

changelog description change

14eb883

jwongmongodb changed the title ~~feat: Supports stream processor modification~~ feat: Adds update support Mar 20, 2025

jwongmongodb requested a review from EspenAlbert March 20, 2025 14:56

cleanup unnecessary comments

6245b70

jwongmongodb requested review from oarbusi and maastha March 20, 2025 15:24

oarbusi approved these changes Mar 20, 2025

View reviewed changes

EspenAlbert reviewed Mar 20, 2025

View reviewed changes

internal/service/streamprocessor/state_transition.go Outdated Show resolved Hide resolved

correctly use the currentState in error message

a043e60

jwongmongodb changed the title ~~feat: Adds update support~~ feat: Adds Stream Processor update support Mar 20, 2025

jwongmongodb requested a review from EspenAlbert March 21, 2025 13:33

update documentation

68b9e69

jwongmongodb requested a review from davidhou17 March 24, 2025 14:49

EspenAlbert approved these changes Mar 24, 2025

View reviewed changes

AgustinBettati reviewed Mar 24, 2025

View reviewed changes

move the stream processor update doc to the correct resources template

9b35f86

jwongmongodb requested a review from AgustinBettati March 24, 2025 15:47

jwongmongodb merged commit 734a939 into master Mar 25, 2025
39 of 40 checks passed

jwongmongodb deleted the CLOUDP-301766 branch March 25, 2025 15:57

svc-apix-Bot added a commit that referenced this pull request Mar 25, 2025

chore: Updates CHANGELOG.md for #3180

c6981fc

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Adds Stream Processor update support #3180

feat: Adds Stream Processor update support #3180

jwongmongodb commented Mar 18, 2025 •

edited

Loading

jwongmongodb left a comment

jwongmongodb Mar 18, 2025

oarbusi Mar 18, 2025

jwongmongodb Mar 18, 2025

jwongmongodb Mar 18, 2025

oarbusi Mar 18, 2025

oarbusi left a comment

oarbusi Mar 18, 2025

jwongmongodb Mar 19, 2025 •

edited

Loading

maastha Mar 20, 2025

jwongmongodb Mar 20, 2025

oarbusi Mar 20, 2025

maastha Mar 21, 2025 •

edited

Loading

jwongmongodb Mar 23, 2025

oarbusi Mar 18, 2025

oarbusi Mar 18, 2025

github-actions bot commented Mar 19, 2025

jasonrydberg left a comment

maastha Mar 20, 2025

maastha Mar 20, 2025 •

edited

Loading

jwongmongodb Mar 20, 2025

maastha Mar 21, 2025

lantoli Mar 20, 2025

lantoli commented Mar 20, 2025

EspenAlbert left a comment

AgustinBettati left a comment

AgustinBettati Mar 24, 2025

jwongmongodb Mar 24, 2025

hshiozawa commented Apr 16, 2025

		@@ -0,0 +1,3 @@
		```release-note:enhancement
		resource/mongodbatlas_stream_processor: Support modifying a stream processor

		@@ -2,6 +2,11 @@

		`mongodbatlas_stream_processor` describes a stream processor.

		NOTE: When updating an Atlas Stream Processor, the following behavior applies:

feat: Adds Stream Processor update support #3180

feat: Adds Stream Processor update support #3180

Conversation

jwongmongodb commented Mar 18, 2025 • edited Loading

Description

Type of change:

Required Checklist:

Further comments

jwongmongodb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oarbusi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jwongmongodb Mar 19, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maastha Mar 21, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Mar 19, 2025

jasonrydberg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maastha Mar 20, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lantoli commented Mar 20, 2025

EspenAlbert left a comment

Choose a reason for hiding this comment

AgustinBettati left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hshiozawa commented Apr 16, 2025

jwongmongodb commented Mar 18, 2025 •

edited

Loading

jwongmongodb Mar 19, 2025 •

edited

Loading

maastha Mar 21, 2025 •

edited

Loading

maastha Mar 20, 2025 •

edited

Loading