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

NO-ISSUE Extend BMAC documentation #1772

Merged
merged 1 commit into from
May 21, 2021

Conversation

mkowalski
Copy link
Contributor

This PR extends BMAC documentation to include all the steps required to
create a fully functional assisted service deployment. It collects links
to various CRDs described in this repository and outlines a step-by-step
process of deploying them.

Some additional debugging tips&tricks are added here&there.

This PR extends BMAC documentation to include all the steps required to
create a fully functional assisted service deployment. It collects links
to various CRDs described in this repository and outlines a step-by-step
process of deploying them.

Some additional debugging tips&tricks are added here&there.
@openshift-ci openshift-ci bot requested review from ronniel1 and slaviered May 20, 2021 09:39
Copy link
Member

@djzager djzager left a comment

Choose a reason for hiding this comment

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

Looks reasonable to me. Will give this a go tomorrow if I can.

/cc @flaper87

since I think he is more familiar with this.

@openshift-ci openshift-ci bot requested a review from flaper87 May 20, 2021 19:54
Copy link
Contributor

@flaper87 flaper87 left a comment

Choose a reason for hiding this comment

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

Love this docs, thank you!

I proposed a few things that we could change in dev-script directly. I think we should still keep all these docs as they are, even when the dev-script changes are applied.

(although, maybe the step related to the disks could be removed)

Comment on lines +59 to +70
```bash
export CLUSTERNAME=ostest
export VMNAME=master_0
export DISK=sdb

qemu-img create -f raw /home/dev-scripts/pool/${VMNAME}_manual_${DISK}.img 10G

virsh attach-disk ${CLUSTERNAME}_${VMNAME} \
--source /home/dev-scripts/pool/${VMNAME}_manual_${DISK}.img \
--target ${DISK} \
--persistent
```
Copy link
Contributor

Choose a reason for hiding this comment

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

I have a small counter-proposal for this. What do you think about taking @lranjbar dev-script PR over so that we won't need to do this? openshift-metal3/dev-scripts#1230

@@ -136,7 +188,7 @@ spec:
credentialsName: bmc-secret
```

Setting `automatedCleaningMode` field and the `inspect.metal3.io` annotations are both optional. If
Setting `automatedCleaningMode` field and the `inspect.metal3.io` is optional. If
Copy link
Contributor

Choose a reason for hiding this comment

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

Along the lines of my previous comment, what do you think about having the assisted_deployment script in dev-script prepare/modify the BareMetalHost manifest for us (starting from the one created by dev-scripts)?

Creating AgentClusterInstall, ClusterDeployment and InfraEnv resources
===

A number of resources has to be created in order to have the deployment fully ready for deploying OCP clusters. A typical workflow is as follows
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
A number of resources has to be created in order to have the deployment fully ready for deploying OCP clusters. A typical workflow is as follows
A number of resources have to be created in order to have the deployment fully ready for deploying OCP clusters. A typical workflow is as follows

* create the [PullSecret](crds/pullsecret.yaml)
* in order to create it directly from file you can use the following
```
kubectl create secret -n assisted-installer generic pull-secret --from-file=.dockerconfigjson=pull_secret.json
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can also have the assisted_deployment target take care of this :)

@openshift-ci
Copy link

openshift-ci bot commented May 21, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: flaper87, mkowalski

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 openshift-ci bot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels May 21, 2021
@flaper87
Copy link
Contributor

/retest

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

2 similar comments
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit 5950aca into openshift:master May 21, 2021
@mkowalski mkowalski deleted the 101-docs branch May 21, 2021 13:51
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. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants