Skip to content

Provisioning updates. #5361

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

Merged
merged 1 commit into from
Oct 18, 2017
Merged

Conversation

kwoodson
Copy link
Contributor

@kwoodson kwoodson commented Sep 11, 2017

These are provisioning updates and features that allow node bootstrapping.

List of updates:

  • Removed the need to specify openshift_node_bootstrap in ami creation.
  • Skip failures in evaluate groups if no etcd hosts are defined
  • More default variables for the openshift_aws role
    • Termination policy for scale groups
    • replace instances or replace all
    • label support
    • ability to run bootstrap.yml on startup
    • moved the logic of the user_data into a template
    • remove any openshift.fact files because ami creation and where the node ends up are different
    • install bootstrap.yml into AMI
    • variable to turn on/off - run the bootstrap.yml on start
    • install openshift-ansible-roles into AMI for later boot use (use yedit to modify files)
    • override the user_data if desired
  • Added the feature to create a configmap per node group
  • Added default variables for openshift_master to support node group configmaps
    • config dir
    • cloud provider
    • node-config settings
      • kubeletargs
      • node labels

Instead of using a template for the node-config.yaml, we call oc adm create-node-config. We have a list of edits that occur to clean up the generation before it is specialized for each node group. Once each node group has specialized their own copy they are placed in the kube-system namespace with the name node-config- configmap.

This is going to require code changes from @smarterclayton to take the configmap and lay it down as the node-config.yaml.

@openshift-ci-robot openshift-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Sep 11, 2017
@kwoodson
Copy link
Contributor Author

aos-ci-test

@openshift-bot
Copy link

success: "aos-ci-jenkins/OS_3.6_containerized, aos-ci-jenkins/OS_3.6_containerized_e2e_tests" for f6b17c7 (logs)

@openshift-bot
Copy link

success: "aos-ci-jenkins/OS_3.6_NOT_containerized, aos-ci-jenkins/OS_3.6_NOT_containerized_e2e_tests" for f6b17c7 (logs)

@kwoodson
Copy link
Contributor Author

aos-ci-test

@openshift-bot
Copy link

success: "aos-ci-jenkins/OS_3.6_NOT_containerized, aos-ci-jenkins/OS_3.6_NOT_containerized_e2e_tests" for a59a356 (logs)

@openshift-bot
Copy link

success: "aos-ci-jenkins/OS_3.6_containerized, aos-ci-jenkins/OS_3.6_containerized_e2e_tests" for a59a356 (logs)

@kwoodson kwoodson force-pushed the fix_bootstrap_files branch 2 times, most recently from 453e5c6 to dcdc2c7 Compare September 18, 2017 21:46
@openshift-ci-robot openshift-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Sep 18, 2017
@kwoodson kwoodson force-pushed the fix_bootstrap_files branch 2 times, most recently from f4c4291 to 3fc903b Compare September 20, 2017 21:09
@openshift-ci-robot openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Sep 20, 2017
@kwoodson kwoodson changed the title Do not remove files for bootstrap if resolv or dns. Provisioning updates. Sep 20, 2017
@kwoodson
Copy link
Contributor Author

/retest

@kwoodson kwoodson force-pushed the fix_bootstrap_files branch 3 times, most recently from 32cd654 to ff05261 Compare September 22, 2017 18:08
@kwoodson kwoodson self-assigned this Sep 22, 2017
@kwoodson
Copy link
Contributor Author

/retest

@kwoodson
Copy link
Contributor Author

/retest

@kwoodson kwoodson force-pushed the fix_bootstrap_files branch 3 times, most recently from 2c7a78b to 0e43e4e Compare September 27, 2017 13:36
@kwoodson
Copy link
Contributor Author

kwoodson commented Oct 9, 2017

@michaelgugino @sdodson, @smarterclayton
These changes have passed the latest round of provision and install tests.

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 11, 2017
@kwoodson kwoodson force-pushed the fix_bootstrap_files branch from eed1e73 to 8d3ac16 Compare October 11, 2017 17:31
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 11, 2017
@kwoodson kwoodson force-pushed the fix_bootstrap_files branch 2 times, most recently from 6c787e1 to 73bf47f Compare October 12, 2017 19:56
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 12, 2017
@kwoodson kwoodson force-pushed the fix_bootstrap_files branch from 73bf47f to ab59305 Compare October 12, 2017 20:40
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 12, 2017

openshift_master_node_config_networkconfig_mtu: 1450

openshift_master_node_config_kubeletargs_cpu: 500m
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These seem like node group specific settings, applied as defaults to a node-config generated per node group.

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 17, 2017
@kwoodson kwoodson force-pushed the fix_bootstrap_files branch from ab59305 to 6bf2d71 Compare October 17, 2017 13:39
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 17, 2017
@kwoodson kwoodson force-pushed the fix_bootstrap_files branch from 6bf2d71 to 47d2e20 Compare October 17, 2017 19:34
@sdodson
Copy link
Member

sdodson commented Oct 17, 2017

I don't want to block this PR any longer. We definitely need to plan follow up work though.
/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 17, 2017
@smarterclayton
Copy link
Contributor

Yeah, this can continue in subsequent PRs. I have GCE bootstrapping and I'm close to a PR that turns it on for origin's conformance_gce suite, which will stepping stone to doing a full image build -> cluster turn up model.

@kwoodson
Copy link
Contributor Author

@sdodson @smarterclayton,
I'd like to change the order of the installation. I'd like to install the control plane along withe the nodes on the masters, scale up with the nodes, then finish by installing the openshift_hosted playbooks. That way everything comes up in the right order.

content: |
openshift_group_type: {{ openshift_aws_node_group_type }}
{% if openshift_aws_node_group_type != 'master' %}
- path: /etc/origin/node/csr_kubeconfig
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to call this bootstrap.kubeconfig to be consistent with its use and other kubeconfig files. Can be a followup.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, sounds good.

@kwoodson
Copy link
Contributor Author

/test logging


openshift_master_client_binary: "{{ openshift.common.client_binary if openshift is defined else 'oc' }}"

openshift_master_config_imageconfig_format: "{{ oreg_url if oreg_url != '' else 'registry.access.redhat.com/openshift3/ose-${component}:${version}' }}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we already have this in another form?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the one that is used is oreg_url so I stayed with it. I'm open to change here.

@openshift-ci-robot
Copy link

@kwoodson: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
ci/openshift-jenkins/logging 47d2e20 link /test logging

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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.

Copy link
Contributor

@michaelgugino michaelgugino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to see a note in the documentation or in the plays themselves that indicates the provisioning parts are still under active development.

@sdodson
Copy link
Member

sdodson commented Oct 18, 2017

I would like to see a note in the documentation or in the plays themselves that indicates the provisioning parts are still under active development.

I agree, I think we'd forego docs.openshift.com documentation for now but we should make it clear in playbooks/aws/README.md that these are alpha quality.

@sdodson sdodson merged commit 63b77fb into openshift:master Oct 18, 2017
@smarterclayton
Copy link
Contributor

We definitely need to split openshift-cluster/config.yml into three equal playbooks that can be called independently - master, node, and hosted/post-install. Bootstrapping needs master up to get the bootstrap.kubeconfig, and ideally as kenny notes we want to:

  1. provision the control plane physical resources (optionally provision non-bootstrapped nodes)
  2. install the control plane
  3. set up the bootstrapping config for use by bootstrapped nodes
  4. provision bootstrapped node groups AND/OR configure non-bootstrapped nodes
  5. install hosted components once sufficient nodes are available

Is that slated yet?

@kwoodson
Copy link
Contributor Author

@smarterclayton,

I opened up this PR this morning on something I had been testing. #5846.

I'd like to continue this discussion in that PR.

@kwoodson kwoodson deleted the fix_bootstrap_files branch March 5, 2018 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm Indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants