-
Notifications
You must be signed in to change notification settings - Fork 98
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
Multicluster setup (n > 2) #722
Conversation
f1d48f7
to
8ffee2f
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.
probably requires additional investigation,
please consider native iterators (see example):
# search, based on value from first list retrieves value in second list
# l1=[a b c] l2=[A D E] -> search(b, l1,l2) -> D
search = $(subst $1_,,$(filter $1_%, $(join $(addsuffix _,$2),$3)))
k3ds ?= $(shell seq -f 'k3d-%g.yaml' 1 10)
k3dgeo ?= us de za cz es sp nl ru cn eu
https ?= $(shell seq 8053 8063)
k3d-%.yaml:
@echo anycommand $@ $(*) \
$(call search,$@,$(k3ds),$(k3dgeo)) \
$(call search,$@,$(k3ds),$(https))
.PHONY: k3dgen
k3dgen: $(k3ds)
❯ make k3dgen
anycommand k3d-1.yaml 1 us 8053
anycommand k3d-2.yaml 2 de 8054
anycommand k3d-3.yaml 3 za 8055
anycommand k3d-4.yaml 4 cz 8056
anycommand k3d-5.yaml 5 es 8057
anycommand k3d-6.yaml 6 sp 8058
anycommand k3d-7.yaml 7 nl 8059
anycommand k3d-8.yaml 8 ru 8060
anycommand k3d-9.yaml 9 cn 8061
anycommand k3d-10.yaml 10 eu 8062
8ffee2f
to
6c0fcaa
Compare
I know about it, but I wanted to avoid 'dynamic targets' or pattern rules, or what is it called to make it simple. I like the way how you combine two strings together and then allow for "random access" in it so I've borrowed that for the geo tags to have it as one-liner (space separated) instead of those 10 lines and 10 different variables |
062cbfd
to
27738fa
Compare
27738fa
to
7bfe646
Compare
4f7398a
to
0530c74
Compare
c62a134
to
3a731ff
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.
Concern:
The default number of clusters produced by Makefile is 2.
The hardcoded number of clusters in terratest is becoming 3 with this change.
It will break local make terratest
invocation when CLUSTER_NUMBER != 3
We probably should fail fast in make terratest
target if CLUSTER_NUMBER !=3 with a clear message that tests are expecting 3 clusters to avoid confusion in future.
In the long run we might extend terratest suite to be more dynamic in this regards but it will be probably overkilling.
3a731ff
to
a14a046
Compare
- change the Makefile targets so that they run in a for loop - add tests for terratest for round-robin @ 3 clusters (failover is still pending) - change the helm chart to be able to set up more than one hostAlias entry for a pod - add simple script that generates the yaml configs for k3d clusters so that we are DRY Signed-off-by: Jirka Kremser <jiri.kremser@gmail.com>
a14a046
to
94fd1bb
Compare
it's there |
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 fixes, LGTM
Thanks for reviews! terrascan & CodeQL checks are being stucked, but these were passing before, so merging.. |
Only `deploy-full-local-setup` target will deploy podinfo and custom resources. I accidentally reverted this existing logic in pr k8gb-io#722 Signed-off-by: Jirka Kremser <jiri.kremser@gmail.com>
Only `deploy-full-local-setup` target will deploy podinfo and custom resources. I accidentally reverted this existing logic in pr #722 Signed-off-by: Jirka Kremser <jiri.kremser@gmail.com>
to verify the change locally i suggest:
another consequence of this PR is that when trying to incorporate a change in local helm chart (that hasn't been published yet), one can do
make deploy-full-local-setup CHART=./chart/k8gb
Signed-off-by: Jirka Kremser jiri.kremser@gmail.com