Conversation
Signed-off-by: Alex Wu <c.alexwu@gmail.com>
Signed-off-by: Alex Wu <c.alexwu@gmail.com>
Signed-off-by: Alex Wu <c.alexwu@gmail.com>
charts/event.json
Outdated
| @@ -0,0 +1,5 @@ | |||
| { | |||
| "repository": { | |||
| "default_branch": "master" | |||
There was a problem hiding this comment.
What is this file? should we use v2 branch
There was a problem hiding this comment.
Sorry this is an unused file, deleted.
|
Seeing this error when building the sandbox => CACHED [flytebuilder 6/17] COPY flytecopilot flytecopilot 0.0s
=> CACHED [flytebuilder 7/17] COPY flyteidl2 flyteidl2 0.0s
=> CACHED [flytebuilder 8/17] COPY flyteplugins flyteplugins 0.0s
=> CACHED [flytebuilder 9/17] COPY flytestdlib flytestdlib 0.0s
=> CACHED [flytebuilder 10/17] COPY gen gen 0.0s
=> ERROR [flytebuilder 11/17] COPY queue queue 0.0s
=> CACHED [flytebuilder 12/17] COPY runs runs 0.0s
=> ERROR [flytebuilder 13/17] COPY state state 0.0s
------
> [flytebuilder 11/17] COPY queue queue:
------
------
> [flytebuilder 13/17] COPY state state:
------
1 warning found (use docker --debug to expand):
- CopyIgnoredFile: Attempting to Copy file "gen" that is excluded by .dockerignore (line 18)
Dockerfile:21
--------------------
19 | COPY queue queue
20 | COPY runs runs
21 | >>> COPY state state
22 |
23 | COPY go.mod go.sum ./
--------------------
ERROR: failed to build: failed to solve: failed to compute cache key: failed to calculate checksum of ref ekpscsqwcfa3y7gk4uz2qd27c::3cqpgmh56y7uxudzrdnq23t3i: "/state": not found
View build details: docker-desktop://dashboard/build/flyte-sandbox/flyte-sandbox0/lkbrhm8bgsq2h72sjruqmtdll
make[1]: *** [flyte] Error 1
make: *** [build-sandbox] Error 2 |
Signed-off-by: Alex Wu <c.alexwu@gmail.com>
Signed-off-by: Alex Wu <c.alexwu@gmail.com>
@pingsutw The build failed due to state and queue service are deleted. I just push a fix commit to it and the build is working now. Thanks! |
|
Could you also paster the config for flyte-sdk in the PR description? Is it like so? admin:
endpoint: dns:///localhost:8090
insecure: True
image:
builder: local
task:
domain: development
project: testproject
org: testorg
|
|
Got this error when running Downloading docker-registry from repo https://twuni.github.io/docker-registry.helm
Downloading kubernetes-dashboard from repo https://kubernetes-retired.github.io/dashboard/
Downloading minio from repo https://charts.bitnami.com/bitnami
Downloading postgresql from repo https://charts.bitnami.com/bitnami
Deleting outdated charts
mkdir -p manifests
kustomize build \
--enable-helm \
--load-restrictor=LoadRestrictionsNone \
kustomize/complete > manifests/complete.yaml
Error: namespace transformation produces ID conflict: [{"apiVersion":"v1","kind":"Secret","metadata":{"annotations":{"internal.config.kubernetes.io/previousKinds":"Secret","internal.config.kubernetes.io/previousNames":"kubernetes-dashboard-csrf","internal.config.kubernetes.io/previousNamespaces":"flyte"},"name":"kubernetes-dashboard-csrf","namespace":"flyte"}} {"apiVersion":"v1","kind":"Secret","metadata":{"annotations":{"internal.config.kubernetes.io/previousKinds":"Secret","internal.config.kubernetes.io/previousNames":"kubernetes-dashboard-csrf","internal.config.kubernetes.io/previousNamespaces":"default"},"labels":{"app.kubernetes.io/instance":"flyte-sandbox","app.kubernetes.io/managed-by":"Helm","app.kubernetes.io/name":"kubernetes-dashboard","app.kubernetes.io/version":"2.7.0","helm.sh/chart":"kubernetes-dashboard-6.0.0"},"name":"kubernetes-dashboard-csrf","namespace":"flyte"},"type":"Opaque"}]
make[1]: *** [manifests] Error 1
make: *** [build-sandbox] Error 2I think it's because we enable flyte/charts/flyte-sandbox/values.yaml Lines 90 to 91 in 784cd7c And this will create the secret automathically: flyte/docker/sandbox-bundled/manifests/dev.yaml Lines 812 to 822 in 784cd7c Then we defined secret with same name in: flyte/docker/sandbox-bundled/kustomize/dashboard-rbac.yaml Lines 1 to 5 in 784cd7c this cause the conflict |
I have updated the PR description, thanks! |
Strange it runs fine in my local machine. I just changed the way to set k8s dashboard secret, can you help me to try again? |
Signed-off-by: Alex Wu <c.alexwu@gmail.com>
| .PHONY: run-sandbox | ||
| run-sandbox: ## Start the flyte sandbox without rebuilding the image | ||
| $(MAKE) -C docker/sandbox-bundled start | ||
|
|
There was a problem hiding this comment.
Could we add a stop-sandbox command?
| .PHONY: run-sandbox | ||
| run-sandbox: ## Start the flyte sandbox without rebuilding the image | ||
| $(MAKE) -C docker/sandbox-bundled start | ||
|
|
Still get error: kustomize build \
--enable-helm \
--load-restrictor=LoadRestrictionsNone \
kustomize/complete > manifests/complete.yaml
Error: namespace transformation produces ID conflict: [{"apiVersion":"v1","kind":"Service","metadata":{"annotations":{"internal.config.kubernetes.io/previousKinds":"Service","internal.config.kubernetes.io/previousNames":"flyte-sandbox-kubernetes-dashboard","internal.config.kubernetes.io/previousNamespaces":"flyte"},"name":"flyte-sandbox-kubernetes-dashboard","namespace":"flyte"},"spec":{"externalName":"flyte-sandbox-kubernetes-dashboard.default.svc.cluster.local","type":"ExternalName"}} {"apiVersion":"v1","kind":"Service","metadata":{"annotations":{"internal.config.kubernetes.io/previousKinds":"Service","internal.config.kubernetes.io/previousNames":"flyte-sandbox-kubernetes-dashboard","internal.config.kubernetes.io/previousNamespaces":"default"},"labels":{"app.kubernetes.io/component":"kubernetes-dashboard","app.kubernetes.io/instance":"flyte-sandbox","app.kubernetes.io/managed-by":"Helm","app.kubernetes.io/name":"kubernetes-dashboard","app.kubernetes.io/version":"2.7.0","helm.sh/chart":"kubernetes-dashboard-6.0.0","kubernetes.io/cluster-service":"true"},"name":"flyte-sandbox-kubernetes-dashboard","namespace":"flyte"},"spec":{"ports":[{"name":"http","port":80,"targetPort":"http"}],"selector":{"app.kubernetes.io/component":"kubernetes-dashboard","app.kubernetes.io/instance":"flyte-sandbox","app.kubernetes.io/name":"kubernetes-dashboard"},"type":"ClusterIP"}}]
make[1]: *** [manifests] Error 1
make: *** [build-sandbox] Error 2I think it might because we are using the different version? Below is the version I used: # kustomize version
❯ kustomize version
v5.7.1
# helm version
❯ helm version --short
v3.18.6+gb76a950 |
app/k8s.go
Outdated
|
|
||
| func buildRESTConfig(ctx context.Context, kubeconfig string) (*rest.Config, error) { | ||
| if kubeconfig != "" { | ||
| kubeconfig = expandPath(kubeconfig) |
There was a problem hiding this comment.
While this kubeconfig is from the config file, I think it would be better to also add normalize into the config layer, if there's other places that get the kubeconfig, we can also give the normalized path.
We can add util function under flytestdlib/config, and under the GetConfig() for each component, we can do something like so:
func GetConfig() *Config {
raw := configSection.GetConfig().(*Config)
cfg := *raw
cfg.Kubernetes.KubeConfig = config.NormalizeFilePath(cfg.Kubernetes.KubeConfig)
return &cfg
}So that we can ensure that the kubeconfig get from the Config struct is always normalized
|
Just a note for follow-up, Line 44 in b58bbd3 |
|
btw please do: git rm --cached gen/rust/Cargo.lockThis is already being ignored in the |
charts/flyte-binary/README.md
Outdated
| @@ -0,0 +1,175 @@ | |||
| # flyte-binary | |||
|
|
|||
|    | |||
| tasks: | ||
| # -- Plugins configuration, [structure](https://pkg.go.dev/github.com/flyteorg/flytepropeller/pkg/controller/nodes/task/config#TaskPluginConfig) | ||
| task-plugins: | ||
| # -- [Enabled Plugins](https://pkg.go.dev/github.com/lyft/flyteplugins/go/tasks/config#Config). | ||
| # Enable sagemaker*, athena if you install the backend plugins | ||
| enabled-plugins: | ||
| - container | ||
| - sidecar | ||
| - connector-service | ||
| - echo | ||
| default-for-task-types: | ||
| container: container | ||
| sidecar: sidecar | ||
| container_array: k8s-array | ||
| # -- Uncomment to enable task type that uses Flyte Connector | ||
| # bigquery_query_job_task: connector-service |
There was a problem hiding this comment.
Do we need those? I think now we register the plugin through blank import (although we only do blank import for flyteplugins/go/tasks/plugins/k8s/pod now), but I think this is not used anywhere in v2?
There was a problem hiding this comment.
I leave it for now because we might need it in the future I think? e.g. default-for-task-types is crucial when we have multiple plugins for 1 task type and we need to assign the plugin in runtime
| logs: | ||
| config: {{- include "flyte-binary.configuration.logging.plugins" . | nindent 12 }} | ||
| {{- if .Values.flyteconnector.enabled }} | ||
| connector-service: |
There was a problem hiding this comment.
Just a note. Right now we do not call RegisterConnectorPlugin so that this config does not really work. And is there connector in V2? @pingsutw
There was a problem hiding this comment.
We don't have default connector in sandbox in v2, so maybe we don't need this. fixed in ab1ff1e
| 002-database.yaml: | | ||
| {{- with .Values.configuration.database }} | ||
| database: | ||
| postgres: |
There was a problem hiding this comment.
we currently have two options, postgres and sqlite, do we want to include the sqlite config here?
Signed-off-by: Alex Wu <c.alexwu@gmail.com>
There was a problem hiding this comment.
I think this file and charts/flyte-binary/templates/auth-client-secret.yaml are not used anywhere now?
There was a problem hiding this comment.
I just leave it because we might need to implement auth feature in the future
|
I test with ❯ helm template test charts/flyte-binary --set ingress.create=true
Error: template: flyte-binary/templates/ingress/grpc.yaml:2:15: executing "flyte-binary/templates/ingress/grpc.yaml" at <include "flyte-binary.ingress.grpcPaths" .>: error calling include: template: no template "flyte-binary.ingress.grpcPaths" associated with template "gotpl"
Use --debug flag to render out invalid YAML❯ helm template test charts/flyte-binary --set ingress.separateGrpcIngress=false
Error: template: flyte-binary/templates/service/http.yaml:51:15: executing "flyte-binary/templates/service/http.yaml" at <include "flyte-binary.service.grpc.port" .>: error calling include: template: no template "flyte-binary.service.grpc.port" associated with template "gotpl"
Use --debug flag to render out invalid YAML
❯ helm template test charts/flyte-binary --set configuration.auth.enabled=true
Error: template: flyte-binary/templates/auth-client-secret.yaml:5:11: executing "flyte-binary/templates/auth-client-secret.yaml" at <include "flyte-binary.configuration.auth.clientSecretName" .>: error calling include: template: no template "flyte-binary.configuration.auth.clientSecretName" associated with template "gotpl"
Use --debug flag to render out invalid YAML |
| {{- if .Values.commonAnnotations }} | ||
| {{- tpl ( .Values.commonAnnotations | toYaml ) . | nindent 4 }} | ||
| {{- end }} | ||
| {{- if .Values.service.annotations }} |
There was a problem hiding this comment.
We expect service.annotations from Values.yaml, but it's missing in values.yaml. We should either remove it or add the service.annotations under values.yaml
flyte/charts/flyte-binary/values.yaml
Line 311 in 361af15
Signed-off-by: Alex Wu <c.alexwu@gmail.com>
|
Got errors when running ~/w/op/flyte-v2/dock/sandbox-bundled/bootstrap/internal/transform/plugins/config add_flyte_binary_build *22 !2 ?1
❯ go test
# github.com/flyteorg/flyte/docker/sandbox-bundled/bootstrap/internal/transform/plugins/config [github.com/flyteorg/flyte/docker/sandbox-bundled/bootstrap/internal/transform/plugins/config.test]
./loader_test.go:18:3: unknown field ClusterResourceTemplatesConfigMapName in struct literal of type LoaderOpts
./loader_test.go:115:3: unknown field ClusterResourceTemplatesConfigMapName in struct literal of type LoaderOpts
./loader_test.go:125:18: c.clusterResourceTemplates undefined (type *Loader has no field or method clusterResourceTemplates)
./loader_test.go:145:3: unknown field ClusterResourceTemplatesConfigMapName in struct literal of type LoaderOpts
./loader_test.go:155:18: c.clusterResourceTemplates undefined (type *Loader has no field or method clusterResourceTemplates)
FAIL github.com/flyteorg/flyte/docker/sandbox-bundled/bootstrap/internal/transform/plugins/config [build failed]
|
docker/sandbox-bundled/Makefile
Outdated
| --env FLYTE_DEV=$(FLYTE_DEV) \ | ||
| --env K3S_KUBECONFIG_OUTPUT=/.kube/kubeconfig \ | ||
| --volume $(PWD)/.kube:/.kube \ | ||
| --volume $(HOME)/.flyte/sandbox:/var/lib/flyte/config \ |
There was a problem hiding this comment.
We have .flyte/sandbox here and .flyte/sandboxv2 below, should we just pick one?
There was a problem hiding this comment.
We should only mount to .flyte/sandboxv2, fixed
docker/sandbox-bundled/Makefile
Outdated
| . .venv/bin/activate && pip install flytekit | ||
|
|
||
| .PHONY: test | ||
| test: export FLYTECTL_CONFIG := test/config.yaml |
There was a problem hiding this comment.
I think there's no test/ directly under sandbox-bundle?
|
Please do |
Signed-off-by: Alex Wu <c.alexwu@gmail.com>
Signed-off-by: Alex Wu <c.alexwu@gmail.com>
Signed-off-by: Alex Wu <c.alexwu@gmail.com>
Signed-off-by: Alex Wu <c.alexwu@gmail.com>
Fixed in 8d3f05a |
Signed-off-by: Alex Wu <c.alexwu@gmail.com>
Signed-off-by: Alex Wu <c.alexwu@gmail.com>
Signed-off-by: Alex Wu <c.alexwu@gmail.com>
| cd gen/python && uv sync --all-groups --frozen && cd ../.. && | ||
| make gen-local && | ||
| git diff --exit-code || (echo 'Generated files are out of date. Run \`make gen\` and commit changes.' && exit 1) && | ||
| git diff --exit-code -- ':!gen/rust/Cargo.lock' || (echo 'Generated files are out of date. Run \`make gen\` and commit changes.' && exit 1) && |
There was a problem hiding this comment.
We need to add this otherwise the gen diff CI test is gonna fail
Signed-off-by: Alex Wu <c.alexwu@gmail.com>
Signed-off-by: Alex Wu <c.alexwu@gmail.com>
4-6 will be done in follow up issues |
machichima
left a comment
There was a problem hiding this comment.
Overall LGTM! Just note that after I did sandbox-stop and sandbox-run, I'll get this error
❯ make sandbox-stop
Makefile:30: warning: overriding commands for target `build'
go.Makefile:75: warning: ignoring old commands for target `build'
Makefile:76: warning: overriding commands for target `help'
go.Makefile:25: warning: ignoring old commands for target `help'
/Library/Developer/CommandLineTools/usr/bin/make -C docker/sandbox-bundled stop
docker stop flyte-sandbox
flyte-sandbox
❯ make sandbox-run
Makefile:30: warning: overriding commands for target `build'
go.Makefile:75: warning: ignoring old commands for target `build'
Makefile:76: warning: overriding commands for target `help'
go.Makefile:25: warning: ignoring old commands for target `help'
/Library/Developer/CommandLineTools/usr/bin/make -C docker/sandbox-bundled start
[ -n "flyte-sandbox" ] || \
docker volume create flyte-sandbox
docker: Error response from daemon: Conflict. The container name "/flyte-sandbox" is already in use by container "e8b2e7ee4aac95b852516f0bfdb052f0b7d5a7e8630965e6f45784137241c7b0". You have to remove (or rename) that container to be able to reuse that name.
Run 'docker run --help' for more information
make[1]: *** [start] Error 125
make: *** [sandbox-run] Error 2I'll need to manually remove the old container with
docker rm -f flyte-sandbox
Hey I tested it locally multiple times but cannot reproduce the error. I'll merge first and get back to this issue in the future. |
* add helm chart Signed-off-by: Alex Wu <c.alexwu@gmail.com> * add docker folder for running k3s cluster Signed-off-by: Alex Wu <c.alexwu@gmail.com> * delete queue state service add actions service Signed-off-by: Alex Wu <c.alexwu@gmail.com> * delete queue state service add actions service Signed-off-by: Alex Wu <c.alexwu@gmail.com> * patch k8s secret config Signed-off-by: Alex Wu <c.alexwu@gmail.com> * fix unimplement rpc issue Signed-off-by: Alex Wu <c.alexwu@gmail.com> * fix k8s dashboard conflict issue Signed-off-by: Alex Wu <c.alexwu@gmail.com> * add normalize path function to std lib Signed-off-by: Alex Wu <c.alexwu@gmail.com> * add sandbox-stop command and fix sandbox command name Signed-off-by: Alex Wu <c.alexwu@gmail.com> * remove cargo file in gen folder Signed-off-by: Alex Wu <c.alexwu@gmail.com> * fix doc flyte version Signed-off-by: Alex Wu <c.alexwu@gmail.com> * delete connector config in configmap Signed-off-by: Alex Wu <c.alexwu@gmail.com> * add sqlite config Signed-off-by: Alex Wu <c.alexwu@gmail.com> * delete service annotation config Signed-off-by: Alex Wu <c.alexwu@gmail.com> * fix ingress config Signed-off-by: Alex Wu <c.alexwu@gmail.com> * regen manifests Signed-off-by: Alex Wu <c.alexwu@gmail.com> * fix some docker script error Signed-off-by: Alex Wu <c.alexwu@gmail.com> * regenerate manifests Signed-off-by: Alex Wu <c.alexwu@gmail.com> * fix dicker image name Signed-off-by: Alex Wu <c.alexwu@gmail.com> * add kube folder to gitignore Signed-off-by: Alex Wu <c.alexwu@gmail.com> * remove test make script under sandbox bundle folder Signed-off-by: Alex Wu <c.alexwu@gmail.com> * fix test cases Signed-off-by: Alex Wu <c.alexwu@gmail.com> * delete cargo.lock Signed-off-by: Alex Wu <c.alexwu@gmail.com> * regenerate proto Signed-off-by: Alex Wu <c.alexwu@gmail.com> * fix gen diff check rule Signed-off-by: Alex Wu <c.alexwu@gmail.com> * fix gen check rule to exclude Cargo Signed-off-by: Alex Wu <c.alexwu@gmail.com> * delete complete-connector folder Signed-off-by: Alex Wu <c.alexwu@gmail.com> * fix sandbox cluster name Signed-off-by: Alex Wu <c.alexwu@gmail.com> --------- Signed-off-by: Alex Wu <c.alexwu@gmail.com>




Tracking issue
Related to #6922
Why are the changes needed?
What changes were proposed in this pull request?
How was this patch tested?
Build sandbox:
Run sandbox(add
FLYTE_DEV=Truearg to run in dev mode):Set kubeconfig to v2 sandbox
Run an example in flyte-sdk repo:
sdk config.yaml:
The task pods are good to go:
Labels
Please add one or more of the following labels to categorize your PR:
This is important to improve the readability of release notes.
Setup process
Screenshots
Check all the applicable boxes
Related PRs
Docs link
main