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

asset/precluster: pre-cluster stage #1532

Closed
wants to merge 3 commits into from
Closed

asset/precluster: pre-cluster stage #1532

wants to merge 3 commits into from

Conversation

staebler
Copy link
Contributor

@staebler staebler commented Apr 4, 2019

Add an installer command to create a pre-cluster asset. This asset includes read-only copies of assets that are useful prior to creating a cluster.

  • bootstrap ignition
  • master ignition
  • worker ignition
  • metadata.json
  • kubeconfig
  • kube admin password

The files are created in a pre-cluster directory under the assets directory.

The pre-cluster stage is a replacement for the node-configs stage, which had collected more targeted assets than just node configs. The old stage has been retained but deprecated.

A bootstrap-config stage has also been added which targets just the bootstrap ignition.

A bootstrap-config stage has been added which targets just the bootstrap ignition. A node-config stage has been added which targets just the master and worker ignitions.

Fixes https://jira.coreos.com/browse/CORS-948

@openshift-ci-robot openshift-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Apr 4, 2019
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: staebler

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 4, 2019
@staebler
Copy link
Contributor Author

staebler commented Apr 4, 2019

$ openshift-install create node-config
INFO Consuming "Install Config" from target directory
$ tree
.
├── master.ign
└── worker.ign

0 directories, 2 files
$ openshift-install create bootstrap-config
INFO Consuming "Master Ignition Config" from target directory 
INFO Consuming "Worker Ignition Config" from target directory 
$ tree
.
└── bootstrap.ign

0 directories, 1 file
$ openshift-install create pre-cluster
INFO Consuming "Bootstrap Ignition Config" from target directory
$ tree
.
└── pre-cluster
    ├── auth
    │   ├── kubeadmin-password
    │   └── kubeconfig
    ├── bootstrap.ign
    ├── master.ign
    ├── metadata.json
    └── worker.ign

2 directories, 6 files

@abhinavdahiya
Copy link
Contributor

can we move each of these to separate PRs:
21b631a
a9b3803

??

@staebler
Copy link
Contributor Author

staebler commented Apr 5, 2019

can we move each of these to separate PRs:
21b631a
a9b3803

??

@abhinavdahiya Sure. I can. This PR won't work without those two commits, though, so I would still keep them in this PR. Do you have specific concerns about the individual commits where you think some of them may be acceptable and others not?

@abhinavdahiya
Copy link
Contributor

can we move each of these to separate PRs:
21b631a
a9b3803
??

@abhinavdahiya Sure. I can. This PR won't work without those two commits, though, so I would still keep them in this PR. Do you have specific concerns about the individual commits where you think some of them may be acceptable and others not?

I'm hoping we can fix the metadata and missing kubeadmin issue in existing setup before we decide on using the pre-cluster target.

@staebler
Copy link
Contributor Author

staebler commented Apr 5, 2019

On further analysis, the issue addressed by 21b631a8d74c59cec4fe55c82512c9a33ad88900 is not a problem in master. The metadata.json file is not created when create cluster is run after create ignition-configs. However, since the Load function for the Metadata asset returns false, the asset store does not think that the asset is on disk. Consequently, the metadata.json file is not consumed by create cluster is run. The issue only becomes a problem with the pre-cluster stage introduced in this PR. So, I don't know if it makes sense to move 21b631a8d74c59cec4fe55c82512c9a33ad88900 to its own PR.

@staebler
Copy link
Contributor Author

staebler commented Apr 5, 2019

Also, the kubeadmin-password file only needs to be created by the Kubeadmin Password asset instead of the Cluster asset so that it can also be created by the pre-cluster stage. Since a9b38030026efba18c1595dfb543aa0191323936 is only necessary if the pre-cluster stage is added, it doesn't make sense to move a9b38030026efba18c1595dfb543aa0191323936 to its own PR either.

@staebler
Copy link
Contributor Author

staebler commented Apr 5, 2019

21b631a8d74c59cec4fe55c82512c9a33ad88900: #1538
a9b38030026efba18c1595dfb543aa0191323936: #1539

@wking
Copy link
Member

wking commented Apr 8, 2019

21b631a: #1538
a9b3803: #1539

Both landed; can we get a rebase here? :)

@staebler
Copy link
Contributor Author

staebler commented Apr 8, 2019

21b631a: #1538
a9b3803: #1539

Both landed; can we get a rebase here? :)

b319655be...f681b6581

staebler added 3 commits April 8, 2019 21:03
Add an installer command to create a pre-cluster asset. This
asset includes read-only copies of assets that are useful
prior to creating a cluster.

* bootstrap ignition
* master ignition
* worker ignition
* metadata.json
* kubeconfig
* kube admin password

The files are created in a pre-cluster directory under the
assets directory.

The pre-cluster stage is a replacement for the node-configs
stage, which had collected more targeted assets than just
node configs. The old stage has been retained but deprecated.

A bootstrap-config stage has been added which targets
just the bootstrap ignition. A node-config stage has been
added which targets just the master and worker ignitionds.

Fixes https://jira.coreos.com/browse/CORS-948
Regenerate the asset graph to capture the bootstrap-config
and pre-cluster stages.

$ dot -V
dot - graphviz version 2.40.1 (20161225.0304)
The ignition-configs stage has been deprecated and replaced with
the node-config, bootstrap-config, and pre-cluster stages. The docs
are updated to reflect those changes.
@openshift-ci-robot
Copy link
Contributor

@staebler: PR needs rebase.

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.

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 15, 2019
@openshift-ci-robot
Copy link
Contributor

@staebler: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
ci/prow/e2e-aws-upgrade 4c91406 link /test e2e-aws-upgrade
ci/prow/e2e-aws 4c91406 link /test e2e-aws

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.

@abhinavdahiya
Copy link
Contributor

closing due to inactivity. Please reopen if needed.

/close

@openshift-ci-robot
Copy link
Contributor

@abhinavdahiya: Closed this PR.

In response to this:

closing due to inactivity. Please reopen if needed.

/close

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants