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

Local module reference fails when applying multi-clusters #364

Closed
b4nst opened this issue Mar 9, 2024 · 1 comment · Fixed by #429
Closed

Local module reference fails when applying multi-clusters #364

b4nst opened this issue Mar 9, 2024 · 1 comment · Fixed by #429
Labels
area/bundles Timoni's Bundle related issues and pull requests bug Something isn't working

Comments

@b4nst
Copy link
Contributor

b4nst commented Mar 9, 2024

When applying a bundle containing a local module reference with a multi cluster runtime, only the first cluster passes. The second one always fails with this kind of error:

11:55AM ERR module not found at path /var/folders/vl/j712bqm55gd754rr5tdm8bxm0000gn/T/timoni3063563812/gcp-eu/examples/redis

A simple test can be run in this repository by creating this bundle and runtime at the root of the repo:

// bundle.cue
bundle: {
	apiVersion: "v1alpha1"
	name:       "debug"
	instances: {
		redis: {
			module: url: "file://./examples/redis/"
			namespace: "default"
			values: maxmemory: 256
		}
	}
}
// runtime.cue
runtime: {
	apiVersion: "v1alpha1"
	name:       "production"
	clusters: {
		"first": {
			kubeContext: "local"
			group:       "test1"
		}
		"second": {
			kubeContext: "local"
			group:       "test2"
		}
	}
}
timoni bundle apply -f bundle.cue --runtime runtime.cue --diff
@stefanprodan stefanprodan added bug Something isn't working area/bundles Timoni's Bundle related issues and pull requests labels Mar 9, 2024
huguesalary added a commit to huguesalary/timoni that referenced this issue Oct 2, 2024
huguesalary added a commit to huguesalary/timoni that referenced this issue Oct 2, 2024
huguesalary added a commit to huguesalary/timoni that referenced this issue Oct 2, 2024
This bug is caused by the relationship between the following components:
* The `func (b *BundleBuilder) InitWorkspace(workspace string,
  runtimeValues map[string]string) error` function in
  `internal/engine/bundle_builder.go`
* The `BundleBuilder.files` field
* The `BundleBuilder.mapSourceToOrigin` field
* The `func (b *BundleBuilder) getInstanceUrl(v cue.Value) string`
  function in `internal/engine/bundle_builder.go`

When called, the `InitWorkspace` goes through the list of files stored
in `b.files`, parses them and writes the result in new files (let's call
them "result files") located under the path indicated by `workspace`. In
parallel, `b.mapSourceToOrigin` maps a "result file" path to the
original file the result is based on.

At the end of the function, the original list `b.files` is replaced with
the list of "result files".

The next invocation of `InitWorkspace` then uses `b.files` again, but
this time it contains the previously computed "result files" paths.

So, for a given:
* `b.files == ["/path/to/a/bundle.cue"]`

calling `InitWorkspace("/tmp/workspace1/")`, will yield:
* `b.files == ["/tmp/workspace1/bundle.cue"]`
* `b.mapSourceToOrigin["/tmp/workspace1/bundle.cue"]="/path/to/a/bundle.cue"`

Then calling `InitWorkspace("/tmp/workspace2/")`, will yield:
* `b.files == ["/tmp/workspace2/bundle.cue"]`
* `b.mapSourceToOrigin["/tmp/workspace2/bundle.cue"]="/tmp/workspace1/bundle.cue"`

The `getInstanceUrl` relies on the `mapSourceToOrigin` being accurate to
propely resolve relative `file://` paths.

When `getInstanceUrl()` is called for the first bundle, it will propely
resolve `./examples/redis/` to `/path/to/a/examples/redis/`.

When `getInstanceUrl()` is called for the second bundle, it will erroneously
resolve `./examples/redis/` to `/tmp/workspace1/examples/redis/`.

This commit fixes this.

However it is worth noting that using a relative path for a `file://`
URI feels strange. Should it be allowed in the first place?
@huguesalary
Copy link
Contributor

huguesalary commented Oct 2, 2024

Looked into this and created Draft PR #429

I'm not super familiar with Timoni's codebase, so while this fix works, I don't know if it actually makes sense.

edit: Also, I just realized that if timoni bundle apply -f - --runtime runtime.cue --diff is used instead of timoni bundle apply -f bundle.cue --runtime runtime.cue --diff (note the use of -f -), the fix doesn't work.

huguesalary added a commit to huguesalary/timoni that referenced this issue Oct 3, 2024
This bug is caused by the relationship between the following components:
* The `func (b *BundleBuilder) InitWorkspace(workspace string,
  runtimeValues map[string]string) error` function in
  `internal/engine/bundle_builder.go`
* The `BundleBuilder.files` field
* The `BundleBuilder.mapSourceToOrigin` field
* The `func (b *BundleBuilder) getInstanceUrl(v cue.Value) string`
  function in `internal/engine/bundle_builder.go`

When called, the `InitWorkspace` goes through the list of files stored
in `b.files`, parses them and writes the result in new files (let's call
them "result files") located under the path indicated by `workspace`. In
parallel, `b.mapSourceToOrigin` maps a "result file" path to the
original file the result is based on.

At the end of the function, the original list `b.files` is replaced with
the list of "result files".

The next invocation of `InitWorkspace` then uses `b.files` again, but
this time it contains the previously computed "result files" paths.

So, for a given:
* `b.files == ["/path/to/a/bundle.cue"]`

calling `InitWorkspace("/tmp/workspace1/")`, will yield:
* `b.files == ["/tmp/workspace1/bundle.cue"]`
* `b.mapSourceToOrigin["/tmp/workspace1/bundle.cue"]="/path/to/a/bundle.cue"`

Then calling `InitWorkspace("/tmp/workspace2/")`, will yield:
* `b.files == ["/tmp/workspace2/bundle.cue"]`
* `b.mapSourceToOrigin["/tmp/workspace2/bundle.cue"]="/tmp/workspace1/bundle.cue"`

The `getInstanceUrl` relies on the `mapSourceToOrigin` being accurate to
propely resolve relative `file://` paths.

When `getInstanceUrl()` is called for the first bundle, it will propely
resolve `./examples/redis/` to `/path/to/a/examples/redis/`.

When `getInstanceUrl()` is called for the second bundle, it will erroneously
resolve `./examples/redis/` to `/tmp/workspace1/examples/redis/`.

This commit fixes this.

However it is worth noting that using a relative path for a `file://`
URI feels strange. Should it be allowed in the first place?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/bundles Timoni's Bundle related issues and pull requests bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants