Add tilt for debug configuration #45
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: nagygergo The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Hi @nagygergo. Thanks for your PR. I'm waiting for a nephio-project member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
efiacor
requested review from
liamfallon,
efiacor,
kushnaidu and
Catalin-Stratulat-Ericsson
|
/ok-to-test |
|
|
||
| ### What's happening underneath | ||
| For each of the porch components there is a `Dockerfile.debug`, which has 2 significant differences from the original process. | ||
| The application code is compiled with flags so that all inlining of code and other optimalizations done by the compiler are disabled, so the debugger can track source code lines correctly. |
There was a problem hiding this comment.
| The application code is compiled with flags so that all inlining of code and other optimalizations done by the compiler are disabled, so the debugger can track source code lines correctly. | |
| The application code is compiled with flags so that all inlining of code and other optimizations done by the compiler are disabled, so the debugger can track source code lines correctly. |
| ### What's happening underneath | ||
| For each of the porch components there is a `Dockerfile.debug`, which has 2 significant differences from the original process. | ||
| The application code is compiled with flags so that all inlining of code and other optimalizations done by the compiler are disabled, so the debugger can track source code lines correctly. | ||
| A wrapper application is replacing the entrypoint of the container, called [delve](https://github.com/go-delve/delve). This will expose a rpc API called [dap](https://microsoft.github.io/debug-adapter-protocol/overview), which can be used by many IDEs for remote debugging. The ports `:4000`, `:4001`, `:4002` are hardcoded into the respective images. Delve will wait until there is a debugger attached before kicking off the porch application process. |
There was a problem hiding this comment.
| A wrapper application is replacing the entrypoint of the container, called [delve](https://github.com/go-delve/delve). This will expose a rpc API called [dap](https://microsoft.github.io/debug-adapter-protocol/overview), which can be used by many IDEs for remote debugging. The ports `:4000`, `:4001`, `:4002` are hardcoded into the respective images. Delve will wait until there is a debugger attached before kicking off the porch application process. | |
| A wrapper application is replacing the entrypoint of the container, called [delve](https://github.com/go-delve/delve). This will expose an rpc API called [dap](https://microsoft.github.io/debug-adapter-protocol/overview), which can be used by many IDEs for remote debugging. The ports `:4000`, `:4001`, `:4002` are hardcoded into the respective images. Delve will wait until there is a debugger attached before kicking off the porch application process. |
| For each of the porch components there is a `Dockerfile.debug`, which has 2 significant differences from the original process. | ||
| The application code is compiled with flags so that all inlining of code and other optimalizations done by the compiler are disabled, so the debugger can track source code lines correctly. | ||
| A wrapper application is replacing the entrypoint of the container, called [delve](https://github.com/go-delve/delve). This will expose a rpc API called [dap](https://microsoft.github.io/debug-adapter-protocol/overview), which can be used by many IDEs for remote debugging. The ports `:4000`, `:4001`, `:4002` are hardcoded into the respective images. Delve will wait until there is a debugger attached before kicking off the porch application process. | ||
| There is a configuration in `Tiltfile`, which whenever one of the containers are deployed in debug mode, creates a `kubectl port-forward` for the respective port to `localhost`. This means that VS Code can now attach it's remote debugger to the `localhost:400x` port, and start monitoring the running application in k8s. |
There was a problem hiding this comment.
| There is a configuration in `Tiltfile`, which whenever one of the containers are deployed in debug mode, creates a `kubectl port-forward` for the respective port to `localhost`. This means that VS Code can now attach it's remote debugger to the `localhost:400x` port, and start monitoring the running application in k8s. | |
| There is also configuration in `Tiltfile`, which whenever one of the containers are deployed in debug mode, creates a `kubectl port-forward` for the respective port to `localhost`. This means that VS Code can now attach it's remote debugger to the `localhost:400x` port, and start monitoring the running application in k8s. |
| crds = [ | ||
| "api/porchconfig/v1alpha1/config.porch.kpt.dev_functions.yaml", | ||
| "api/porchconfig/v1alpha1/config.porch.kpt.dev_repositories.yaml", | ||
| "controllers/config/crd/bases/config.porch.kpt.dev_fleetmembershipbindings.yaml", |
There was a problem hiding this comment.
The fleet stuff is junk left over from the migration from kpt and needs to be removed/moved to another repo. We don't use it so can be ignored. Same goes for the Function CRD above.
| @@ -37,16 +37,15 @@ spec: | |||
| serviceAccountName: porch-fn-runner | |||
| containers: | |||
| - name: function-runner | |||
| image: gcr.io/example-google-project-id/porch-function-runner:latest | |||
| image: nephio-project/porch-function-runner:latest | |||
There was a problem hiding this comment.
Not sure if this might cause issues with some of the other Make targets.
There was a problem hiding this comment.
Seems like scripts/create-deployment-blueprint.sh is looking for those, for replacing them. Fixed it, but I couldn't get the associated make target working, seems like has some google things in there.
There was a problem hiding this comment.
Ye I think some/most of that google specific stuff should be removed.
I started using the same pkg when doing the kind deploy target but switched to use the catalog pkg instead.
https://github.com/nephio-project/porch/blob/main/Makefile#L325
We need to refine the Make file to reduce the bloat.
| image: gcr.io/example-google-project-id/porch-controllers:latest | ||
| image: nephio-project/porch-controllers:latest | ||
| args: | ||
| - --reconcilers=* |
There was a problem hiding this comment.
This dummy porch pkg is not ideal. We should prob look to remove it and just use the upstream pkg here - https://github.com/nephio-project/catalog/blob/main/nephio/core/porch/9-controllers.yaml
The above will prob enable the fleetsync controller which we don't want.
Some of this logic needs to be refactored also - https://github.com/nephio-project/porch/blob/main/controllers/main.go#L145
Especially this - https://github.com/nephio-project/porch/blob/main/controllers/main.go#L186
There was a problem hiding this comment.
Yes, this package is missing an ClusterRole/RoleBinding for the porch-controllers as well. Do you think if it's a good idea to manage the deployment artifacts outside the repo? Or you want to copy it into this repo?
There was a problem hiding this comment.
It's a messy one, but ideally we have 1 source of truth for the porch pkg. The official repo is the catalog repo.
It does break/confuse the Make/test setup somewhat though.
|
/assign @tliron |
|
@nagygergo Thanks for the great work. I think we should not integrate Tilt into the main codebase but should place this work in the Porch developer tutorial (or a separate tutorial) so that users can set Tilt and in-cluster debugging themselves, as @tliron suggested. If we integrate Tilt supprot into the main codebase, then we will have to provide ongoing support for it and also offer support for any environment for all comers. This is likely to be very difficult to do. If we provide the description of how to set Tilt up and provide example configurations (stating the environment on which the configurations worked), then we can ask users to download and tweak those configurations themselves. I think this is reasonable because any developer that is attempting in-cluster debugging will likely need to be an advanced user and should be capable of tweaking the config. |
Added dockerfiles, scripts and docs on how to do live debugging of the various porch components.
The wrapper server and the kpt function processes are not easily modifiable this way, so those were left out.
If there is need to debug into those, then the attached can be extended.