-
Notifications
You must be signed in to change notification settings - Fork 29
fix: v2 needs packages fully qualified #39
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
Conversation
lets use dev.localhost/dev as prefix Signed-off-by: Vladimír Dudr <[email protected]>
a54488c to
bf6e21e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR 🚀 I've left some comments regarding some (edge) cases, feedbacks are welcome.
| @$(KIND) load docker-image $(BUILD_REGISTRY)/$*-$(ARCH) -n $(KIND_CLUSTER_NAME) | ||
| @echo '{"apiVersion":"pkg.crossplane.io/v1beta1","kind":"DeploymentRuntimeConfig","metadata":{"name":"runtimeconfig-$*"},"spec":{"deploymentTemplate":{"spec":{"selector":{},"strategy":{},"template":{"spec":{"containers":[{"args":["--debug"],"image":"$(BUILD_REGISTRY)/$*-$(ARCH)","name":"package-runtime"}]}}}}}}' | $(KUBECTL) apply -f - | ||
| @echo '{"apiVersion":"pkg.crossplane.io/v1","kind":"Provider","metadata":{"name":"$*"},"spec":{"package":"$*-$(VERSION).gz","skipDependencyResolution": $(XPKG_SKIP_DEP_RESOLUTION), "packagePullPolicy":"Never","runtimeConfigRef":{"name":"runtimeconfig-$*"}}}' | $(KUBECTL) apply -f - | ||
| echo '{"apiVersion":"pkg.crossplane.io/v1","kind":"Provider","metadata":{"name":"$*"},"spec":{"package":"dev.localhost/dev/$*:$(VERSION).gz","skipDependencyResolution": $(XPKG_SKIP_DEP_RESOLUTION), "packagePullPolicy":"Never","runtimeConfigRef":{"name":"runtimeconfig-$*"}}}' | $(KUBECTL) apply -f - |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we might need the same for local.xpkg.deploy.configuration.% target, as it also expect a fully-qualified package.
| @mkdir -p $(XPKG_OUTPUT_DIR)/cache | ||
| @for pkg in $(XPKG_OUTPUT_DIR)/linux_*/*; do $(CROSSPLANE_CLI) xpkg extract --from-xpkg $$pkg -o $(XPKG_OUTPUT_DIR)/cache/$$(basename $$pkg .xpkg).gz; done | ||
| @mkdir -p $(XPKG_OUTPUT_DIR)/cache/dev.localhost/dev/ | ||
| @for pkg in $(XPKG_OUTPUT_DIR)/linux_*/*; do $(CROSSPLANE_CLI) xpkg extract --from-xpkg $$pkg -o $(XPKG_OUTPUT_DIR)/cache/dev.localhost/dev/$$(basename $$pkg .xpkg | sed 's/$(PROJECT_NAME)-/$(PROJECT_NAME):/').gz; done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there are a few edge cases to consider here:
PROJECT_NAMEmight not necessarily be the xpkg name, it can be overridden or project can potentially build multiple xpkgs. AFAIK, an example is the family providers. e.g. the PROJECT_NAME isprovider-awsbut the xpkg is named likeprovider-aws-iam-v1.2.3.dev.xpkgfor a family subpackage. For those, the replacement would be at the wrong place and possibly break it.local.xpkg.syncruns on the whole directory, not only xpkgs built on the current make run.sedmight fail to replace if there are other xpkgs, accumulated from multiple xpkg build invocations.
Maybe we can consider introducing local.xpkg.sync.%.
local.xpkg.deploy.provider.% depends on local.xpkg.sync, and copies the whole cache. It can be potentially scoped to copy only the relevant package for the current invocation. On the other hand, the "deployment" might potentially need multiple packages to be deployed (deps maybe?), therefore need the whole cache to be copied. Not sure we should consider this as a possibility.
Another (not so important) aspect is, if the main project defines a make target that deploys multiple providers (e.g. family providers testing), local.xpkg.deploy.provider.% target is invoked multiple times, causing local.xpkg.sync to be invoked multiple times, re-copying the same xpkgs again for each run. This does not affect the result, though we might save time, not doing the unnecessary sync.
|
Hi, sorry @erhancagirici - I missed notifications about your comments - I guess this is already resolved by #42 . |
lets use dev.localhost/dev as prefix
Fixes: #38