-
Notifications
You must be signed in to change notification settings - Fork 53
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
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…s called from thread-safe functions.
… non-existing (only registered) by the k8s backend.
… - do not remove the k8s service when we just stop the user service.
laurentluce
commented
Jun 9, 2023
...ainer-engine-lib/lib/backend_impls/docker/docker_kurtosis_backend/docker_kurtosis_backend.go
Show resolved
Hide resolved
laurentluce
commented
Jun 9, 2023
.../backend_impls/docker/docker_kurtosis_backend/user_services_functions/start_user_services.go
Show resolved
Hide resolved
laurentluce
commented
Jun 9, 2023
...ib/lib/backend_impls/kubernetes/kubernetes_kurtosis_backend/shared_helpers/shared_helpers.go
Show resolved
Hide resolved
laurentluce
commented
Jun 9, 2023
...mpls/kubernetes/kubernetes_kurtosis_backend/user_services_functions/destroy_user_services.go
Show resolved
Hide resolved
laurentluce
commented
Jun 9, 2023
..._impls/kubernetes/kubernetes_kurtosis_backend/user_services_functions/start_user_services.go
Show resolved
Hide resolved
laurentluce
commented
Jun 9, 2023
container-engine-lib/lib/backend_impls/kubernetes/kubernetes_manager/kubernetes_manager.go
Show resolved
Hide resolved
laurentluce
commented
Jun 9, 2023
container-engine-lib/lib/backend_impls/kubernetes/kubernetes_manager/kubernetes_manager.go
Show resolved
Hide resolved
laurentluce
commented
Jun 9, 2023
core/server/api_container/server/service_network/default_service_network.go
Show resolved
Hide resolved
laurentluce
commented
Jun 9, 2023
laurentluce
commented
Jun 9, 2023
container-engine-lib/lib/backend_impls/docker/docker_manager/docker_manager.go
Show resolved
Hide resolved
laurentluce
commented
Jun 9, 2023
core/server/api_container/server/service_network/default_service_network.go
Show resolved
Hide resolved
laurentluce
commented
Jun 9, 2023
.../backend_impls/docker/docker_kurtosis_backend/user_services_functions/start_user_services.go
Show resolved
Hide resolved
gbouv
approved these changes
Jun 9, 2023
.../backend_impls/docker/docker_kurtosis_backend/user_services_functions/start_user_services.go
Show resolved
Hide resolved
core/server/api_container/server/service_network/default_service_network.go
Show resolved
Hide resolved
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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
Add
plan.start_service
andplan.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/
andcli/
are related to the renaming of the API methodStartServices
toAddServices
. 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 createdServices
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:
Is this change user facing?
YES
References (if applicable):
#673