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

RHEL 7.2 installation support #568

Merged
merged 1 commit into from
Dec 20, 2017
Merged

RHEL 7.2 installation support #568

merged 1 commit into from
Dec 20, 2017

Conversation

sushilrai
Copy link
Contributor

  • Added support for additional packages
  • Cleanup of non-installation disks

Supporting PR for RackHD/on-taskgraph#343

gavin-scott added a commit to gavin-scott/rackhd-packaging that referenced this pull request Dec 6, 2017
ScaleIO storage-only nodes on RHEL require custom RPM packages to be
installed and to have disk partitions cleared. This adds that support
into the RackHD Graph.InstallRHEL workflow. The changes have been
submitted upstream as:

- RackHD/on-taskgraph#343
- RackHD/on-tasks#568
@anhou anhou added the run-test label Dec 7, 2017
"description": "a flag to indicate whether partitions of non-installation should be cleared",
"type": "boolean"
},
"Kdump": {
Copy link
Member

@anhou anhou Dec 7, 2017

Choose a reason for hiding this comment

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

Adding a field 'Kdump' is not a general way, if others want to add other items want to disable/enable by systemctl, more fields will be added, it's not so good. so either controlling it by puppet/ansible/.. after os installation, or adding a more general way for things which could be controlled by 'systemctl'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion. Removed kdump from the PR. Will add support for providing the list of services that needs to be enabled / disabled during the post-installation.

},
"uniqueItems": true
},
"ClearDisk": {
Copy link
Member

Choose a reason for hiding this comment

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

I didn't see 'clearDisk' related codes in RackHD/on-taskgraph#343.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed 'clearDisk' from the PR. Observed some failures after rebooting the server due to 'dd' of non-installation disks. Investigating other options. Will push a separate PR for cleanup of non-OS installation disks

gavin-scott added a commit to gavin-scott/rackhd-packaging that referenced this pull request Dec 14, 2017
ScaleIO storage-only nodes on RHEL require custom RPM packages to be
installed and to have disk partitions cleared. This adds that support
into the RackHD Graph.InstallRHEL workflow. The changes have been
submitted upstream as:

- RackHD/on-taskgraph#343
- RackHD/on-tasks#568
gavin-scott added a commit to gavin-scott/rackhd-packaging that referenced this pull request Dec 14, 2017
ScaleIO storage-only nodes on RHEL require custom RPM packages to be
installed and to have disk partitions cleared. This adds that support
into the RackHD Graph.InstallRHEL workflow. The changes have been
submitted upstream as:

- RackHD/on-taskgraph#343
- RackHD/on-tasks#568
gavin-scott added a commit to gavin-scott/rackhd-packaging that referenced this pull request Dec 14, 2017
ScaleIO storage-only nodes on RHEL require custom RPM packages to be
installed and to have disk partitions cleared. This adds that support
into the RackHD Graph.InstallRHEL workflow. The changes have been
submitted upstream as:

- RackHD/on-taskgraph#343
- RackHD/on-tasks#568
@@ -50,6 +50,9 @@
"networkDevices": {
"$ref": "types-installos.json#/definitions/NetworkDeviceArray"
},
"bonds": {
Copy link
Member

@anhou anhou Dec 15, 2017

Choose a reason for hiding this comment

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

@sushilrai I re-think about 'bonds' field, maybe 'bonds' is also one type of networkDevices interfaces, right? so 'bond' could be at the same level with 'eth0' 'eth1' , and it could be one value of 'device' under 'networkDevices' http://rackhd.readthedocs.io/en/latest/rackhd/install_os.html#networkdevices, right?
If I missunderstand something, please point out

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Bonds networks are defined separately for simplicity. These logical interfaces requires different configuration parameters as compared to normal physical interfaces.
Also we need to configure the bond interfaces before the normal interfaces to avoid any conflict.

@anhou please confirm if we need to include the bond interfaces in networkDevices then I will make necessary changes

Copy link
Member

Choose a reason for hiding this comment

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

@sushilrai make sense for me that it's logical device that has different configurations. +1

"type": "string"
},
"nics": {
"description": "the ipv4 configuration for this interface",
Copy link
Member

Choose a reason for hiding this comment

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

I guess it's copied from 'ipv4', it could be refined

@anhou
Copy link
Member

anhou commented Dec 18, 2017

Could you also help update the document? the docs repo is https://github.com/RackHD/docs. I think at least these two parts could be included. 1. include RHEL 7.2 support in the OS support table at http://rackhd.readthedocs.io/en/latest/rackhd/install_os.html#supported-os-installation-workflows, and update the payloads table at http://rackhd.readthedocs.io/en/latest/rackhd/install_os.html#non-windows-os-installation-workflow-payload, because some fields are only avalable at RHEL/CentOS, so please also help note (RHEL/CentOS only) in the docs. If you have any questions, let me know

@sushilrai
Copy link
Contributor Author

PR RackHD/docs#419 is submitted with documentation change

@anhou
Copy link
Member

anhou commented Dec 19, 2017

Thanks @sushilrai !
by the way, I left a comment about 'nics' 's description last time. I believe it's the only left things. I'll merge this PR after it's fixed :)

}
];
job._validateOptions();
expect(job.options.networkDevices.length).to.equal(1)

Choose a reason for hiding this comment

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

Missing semicolon.

}
];
job._validateOptions();
expect(job.options.networkDevices.length).to.equal(1)

Choose a reason for hiding this comment

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

Missing semicolon.

-  Added support for additional packages
-  NIC Bond
-  MTU configuration
-  Configuring IP gateway as option to non-routable networks
-  Enable / Disable list of services
@sushilrai
Copy link
Contributor Author

@anhou I have added change for OS installation and have added relevant tests to cover new changes. As per test coverage, we are having lesser coverage for base installation. Please let me know if we are good to merge this PR or I need to add some tests to lib/jobs/base-job.js

@anhou
Copy link
Member

anhou commented Dec 20, 2017

@sushilrai we're looking at why concourse-ci fail, that's blocking the merge.

@anhou anhou merged commit 3b4a92e into RackHD:master Dec 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants