Skip to content
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

feat: Start and Stop service Starlark instructions for Docker #694

Merged
merged 27 commits into from Jun 9, 2023

Conversation

laurentluce
Copy link
Contributor

@laurentluce laurentluce commented Jun 9, 2023

Description:

Add plan.start_service and plan.stop_service Starlark instructions for the Docker backend. Support for the K8S backend first requires an update to the Gateway to manage the port forwarder connection during stop and start operations. The k8s backend code itself is ready and the changes (small) are in this PR.

No new backend methods are introduced, the goal being to re-use the current code as much as possible. All the changes under api/ and cli/ are related to the renaming of the API method StartServices to AddServices. Most of the code added is unit and integration tests. The map of service registrations at the service network layer is used as part of the existing start service process so I am using this map to store the service status and config required for the restart process. I thought about using a map of created Services but it is not as clean as leveraging the map of service registrations.

Added some PR comments to help with the review.

Follow-up work in subsequent PRs:

  • Update gateway to manage the port forwarder connection during stop and start operations and enable support for k8s.
  • Update documentation.

Is this change user facing?

YES

References (if applicable):

#673

… non-existing (only registered) by the k8s backend.
… - do not remove the k8s service when we just stop the user service.
@laurentluce laurentluce requested a review from gbouv June 9, 2023 05:10
@laurentluce laurentluce merged commit 10b6b91 into main Jun 9, 2023
27 checks passed
@laurentluce laurentluce deleted the laurent/start_stop_service branch June 9, 2023 16:27
leoporoli pushed a commit that referenced this pull request Jun 12, 2023
🤖 I have created a release *beep* *boop*
---


##
[0.78.0](0.77.4...0.78.0)
(2023-06-12)


### ⚠ BREAKING CHANGES

* Added `main-file` and `main-function-name` flags to the `kurtosis run`
CLI command. These new options were also added in the
`RunStarlarkScript`, `RunStarlarkPackage` and the
`RunStarlarkRemotePackage` SDKs methods, users will have to update the
calls. ([#693](#693))

### Features

* Added `main-file` and `main-function-name` flags to the `kurtosis run`
CLI command. These new options were also added in the
`RunStarlarkScript`, `RunStarlarkPackage` and the
`RunStarlarkRemotePackage` SDKs methods, users will have to update the
calls. ([#693](#693))
([1693237](1693237))
* random function args
([#703](#703))
([e650a20](e650a20))
* Start and Stop service Starlark instructions for Docker
([#694](#694))
([10b6b91](10b6b91))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: kurtosisbot <kurtosisbot@users.noreply.github.com>
laurentluce added a commit that referenced this pull request Jun 23, 2023
## Description:
Most of the work for the K8S backend was done as part of #694. In
addition, the port forwarder reconnect feature reduced the amount of
work we had to finish. Here are the changes made in this PR:
- We keep the K8S service untouched when we stop the user service so it
can be restarted easily by creating a new pod. This change does not
impact the remove service expected behavior.
- We don't try to create a port forwarder connection if the service is
stopped.

A service stopped here is a k8s service with no pod running. I think it
is time to return the service status (registered, started, stopped) as
part of the GetServices response so we can simplify the second item
above. I will do that in a follow-up PR.

## Is this change user facing?
YES

## References (if applicable):
Closes #673 
#694 
#736
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants