-
Notifications
You must be signed in to change notification settings - Fork 787
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
[wip] multi cluster support #6664
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: If they are not already assigned, you can assign the PR to them by writing The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/test images |
/build images |
bf1e2d8
to
6946ad5
Compare
// lets make sure we have the secrets defined as an env var | ||
secretsYaml := os.Getenv("JX_SECRETS_YAML") | ||
if secretsYaml == "" { | ||
return fmt.Errorf("no $JX_SECRETS_YAML environment variable defined.\nPlease point this at your 'secrets.yaml' file.\nSee https://github.com/jenkins-x/enhancements/blob/master/proposals/2/docs/getting-started.md#setting-up-your-secrets\n") |
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.
error strings should not be capitalized or end with punctuation or a newline
// lets make sure we have the secrets defined as an env var | ||
secretsYaml := os.Getenv("JX_SECRETS_YAML") | ||
if secretsYaml == "" { | ||
return fmt.Errorf("no $JX_SECRETS_YAML environment variable defined.\nPlease point this at your 'secrets.yaml' file.\nSee https://github.com/jenkins-x/enhancements/blob/master/proposals/2/docs/getting-started.md#setting-up-your-secrets\n") |
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.
error strings should not be capitalized or end with punctuation or a newline
So, I was reading https://github.com/jenkins-x/enhancements/blob/master/proposals/2/docs/multi-cluster.md and I had a couple of questions:
Thanks so much for improving multi-cluster support. The proposal looks like a very good improvement over the environment controller! 👍 |
|
Hey @jstrachan - sorry, I cannot figure out how to send feedback on other parts of your proposal, so I hope you don't mind I jot them down here. Questions are as follows:
Thanks! This is all very exciting. Cannot believe you are already taking this way beyond POC, you are a machine! 🚀 💯 |
Right there with you on building images in dev only. This is really only PRs for the environment, where you bump the version of AppFoo to 1.2.3. |
@polothy thanks! Any feedback anywhere is great ;)
It might be simpler to let the Incidentally the system charts are for dealing with ingress/DNS/certs/TLS. Most apps (the |
+1 |
@rawlingsj maybe we could add a final pipeline step of Mind you it’s trivial to do that yourself via |
actually I think having a |
We currently install many cluster wide tools using a Totally right that we can just modify the pipeline and do a Starting to get the impression that the ---Switching topics here--- I did notice this helmfile and it seems like it isn't being used. It looks like from the pipeline that the system and app helmfiles are generated, then the system helmfile is run, a JX command is run and then the apps helmfile is run. I was wondering if you are interested in the helmfile hooks? The thought being, you could use the hook to run the JX command like so: - name: NAME
namespace: NAMESPACE
chart: foo/bar
version: 0.1.0
atomic: true
wait: true
labels:
initPhase: "true"
needs:
- some_other/chart
hooks:
- events: ["postsync"]
showlogs: true
command: jx
args:
- step
- create
- install
- values
# And so on... I'm not even close to knowing if that would work for you but, this would keep all the steps in the helmfile and maybe we could use that root helmfile to control the whole sync, like so: helmDefaults:
# values, values, values...
helmfiles:
- helmfiles/my_crazy_init.yaml
- system/helmfile.yaml
- apps/helmfile.yaml
- helmfiles/cluster_tools.yaml
- helmfiles/rule_the_world.yaml That way JX can own/manage the lifecycle of the system and app helmfiles and we can shove in our stuff where we need to rather easily. Pipeline is just a single Just bouncing ideas around, like I said, this is hot stuff, I like what you have already and where you are taking it 🍻 |
d1ccee5
to
da1ea5a
Compare
// lets make sure we have the secrets defined as an env var | ||
secretsYaml := os.Getenv("JX_SECRETS_YAML") | ||
if secretsYaml == "" { | ||
return fmt.Errorf("no $JX_SECRETS_YAML environment variable defined.\nPlease point this at your 'secrets.yaml' file.\nSee https://github.com/jenkins-x/enhancements/blob/master/proposals/2/docs/getting-started.md#setting-up-your-secrets\n") |
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.
error strings should not be capitalized or end with punctuation or a newline
// lets make sure we have the secrets defined as an env var | ||
secretsYaml := os.Getenv("JX_SECRETS_YAML") | ||
if secretsYaml == "" { | ||
return fmt.Errorf("no $JX_SECRETS_YAML environment variable defined.\nPlease point this at your 'secrets.yaml' file.\nSee https://github.com/jenkins-x/enhancements/blob/master/proposals/2/docs/getting-started.md#setting-up-your-secrets\n") |
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.
error strings should not be capitalized or end with punctuation or a newline
// lets make sure we have the secrets defined as an env var | ||
secretsYaml := os.Getenv("JX_SECRETS_YAML") | ||
if secretsYaml == "" { | ||
return fmt.Errorf("no $JX_SECRETS_YAML environment variable defined.\nPlease point this at your 'secrets.yaml' file.\nSee https://github.com/jenkins-x/enhancements/blob/master/proposals/2/docs/getting-started.md#setting-up-your-secrets\n") |
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.
error strings should not be capitalized or end with punctuation or a newline
/build images |
@polothy its totally fine for folks to do vanilla helmfile stuff; but hopefully we can convince folks to use Jenkins X Apps for lots of common things like nginx/gloo/istio/flagger/prometheus and the like. Mostly as lots of these tools need to be configured with secrets + domain/TLS/certs stuff; so we can figure out that binding between an off-the-shelf helm chart and the In terms of the top level helmfile.yaml thats mostly so you can easily uninstall everything in one simple command: https://github.com/jenkins-x/enhancements/blob/master/proposals/2/docs/getting-started.md#uninstall incidentally this git repo is a cleaner example of what boot config looks like with helmfile: https://github.com/jenkins-x/jenkins-x-boot-helmfile-config (its the git repo used if you run Thanks for the heads up on helm hooks, they look great. We could take advantage of those for sure; though I'm also liking the flexibility we get with pipelines. e.g. by default the pipeline steps are defined in the build pack: https://github.com/jenkins-x-buildpacks/jenkins-x-kubernetes/blob/master/packs/environment-helmfile/pipeline.yaml#L18 so that the default |
until we get this PR merged jenkins-x/jx#6664
/build images |
1 similar comment
/build images |
as we want to create and manage all k8s resources via helm instead Signed-off-by: James Strachan <[email protected]>
also add validation of the enviroment variable `JX_SECRETS_YAML` to ensure its defined to point to the secrets yaml file Signed-off-by: James Strachan <[email protected]>
so that we handle the public chart museum URL and include it into the `jx-apps.yml` file in the pull request Signed-off-by: James Strachan <[email protected]>
so its easier for folks to populate it. Longer term we can help the user populate it like we do with the current boot approach
rename the `jx-apps.yml` apps.valueFiles to apps.values to match helmfile's naming convention also allow discovery of a `myrepo/mychart`'s yaml file via `mychart/values.yaml` as using `myrepo/mychart/values.yaml` seems to confuse helm into thinking the `myrepo/mychart` folder is a local chart source code
if we have no releases for a helmfile lets add a dummy empty chart to avoid `helmfile sync` failing in the system or apps dir refactored the test cases so they are easier to create
lets lazily create the `jx-requirements.values.yaml.gotmpl` file if it does not exist. This lets us use a simpler PR pipeline on local environments without needing to run the whole `jx step verify preinstall` step. also lets default the `--values` arguments with nice validation if folks don't specify them so we can simplify the OOTB pipelines
which it usually does on helm 2 containers
…pressions e.g. so we can enable helm 3 in a build pack via `PATH=/opt/bin/helm3-bin:$PATH`
if we are using helmfile for dev
…ues running steps and capturing system out
if we have configured helm 3 then lets use that version to validate helm Signed-off-by: James Strachan <[email protected]>
so that we can reuse the same code in jx alpha / jx 3.x where we default to helm 3 Signed-off-by: James Strachan <[email protected]>
Signed-off-by: James Strachan <[email protected]>
b692131
to
2e16696
Compare
Signed-off-by: James Strachan <[email protected]>
@jstrachan: The following tests failed, say
View all Builds for this Pull Request 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. I understand the commands that are listed here. |
I'm gonna close this PR for now - we're using this approach for folks to test out multi cluster: https://jenkins-x.io/docs/labs/boot/ |
note this PR depends on the helmfile + apps support: #6660 - once that merges we can rebase this PR (which will then be quite small ;)
for documentation see https://github.com/jenkins-x/enhancements/tree/master/proposals/2/docs