Skip to content
This repository has been archived by the owner on Jul 30, 2024. It is now read-only.

Added a container cache disk option. #149

Merged
merged 2 commits into from
Jul 9, 2021
Merged

Added a container cache disk option. #149

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cd0034a
Added a container cache disk option.
rmkraus Jul 2, 2021
9e0086f
Changed openshift-installer role to be better ansible.
rmkraus Jul 9, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion app/inventory.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,8 @@ def main(config, ipam, inv):
proxy_http=config.get('PROXY_HTTP', ''),
proxy_https=config.get('PROXY_HTTPS', ''),
proxy_noproxy=[item['dest'] for item in json.loads(config.get('PROXY_NOPROXY', '[]'))],
proxy_ca=config.get('PROXY_CA', ''))
proxy_ca=config.get('PROXY_CA', ''),
cache_disk=config.get('CACHE_DISK', ''))

inv.add_host('localhost',
ansible_connection='local',
Expand Down
4 changes: 3 additions & 1 deletion app/lib/python/conftui.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ def __init__(self, name, prompt, disabled=False, default=None):
self._name = name
self._value = os.environ.get(self._name, default or '')
self._prompt = prompt
self._default = default
self.disabled = disabled

@property
Expand All @@ -28,7 +29,8 @@ def value(self):

@value.setter
def value(self, value):
self._value = default if default and not value.strip() else value
self._value = self._default \
if self._default and not value.strip() else value

@property
def prompt(self):
Expand Down
4 changes: 2 additions & 2 deletions app/playbooks/config.d/cluster/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ def __init__(self, path, footer, rtr_interfaces):
[('name', 'Node Name'), ('mac', 'MAC Address'),
('mgmt_mac', 'Management MAC Address'),
('install_drive', 'OS Install Drive',
os.environ.get('BOOT_DRIVE'))])
])
os.environ.get('BOOT_DRIVE'))]),
Parameter('CACHE_DISK', 'Container Cache Disk')])
self.extra = ParameterCollection('extra', 'Extra DNS/DHCP Records', [
ListDictParameter('EXTRA_NODES', 'Static IP Reservations',
[('name', 'Node Name'), ('mac', 'MAC Address'), ('ip', 'Requested IP Address')]),
Expand Down
1 change: 1 addition & 0 deletions app/playbooks/create.d/install-repos/create.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
openshift_installer_control_plane: '{{ groups.control_plane }}'
openshift_installer_ssh_key: '{{ lookup("file", ansible_ssh_private_key_file + ".pub") }}'
openshift_installer_fips_mode: "{{ fips_mode }}"
openshift_installer_cache_disk: "{{ cache_disk }}"
openshift_installer_pull_secret: '{{ pull_secret | to_json }}'
openshift_installer_version: "{{ lookup('ini', 'installer section=cluster file=/app/versions.ini') }}"
openshift_installer_proxy: "{{ proxy }}"
Expand Down
1 change: 1 addition & 0 deletions app/roles/openshift-installer/defaults/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ openshift_installer_cluster_id: ocp
openshift_installer_cluster_cidr: 10.128.0.0/14
openshift_installer_service_cidr: 172.30.0.0/16
openshift_installer_fips_mode: False
openshift_installer_cache_disk: ''
openshift_installer_pull_secret: {}
openshift_installer_ssh_key: ''
openshift_installer_proxy: False
Expand Down
29 changes: 24 additions & 5 deletions app/roles/openshift-installer/tasks/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,31 @@
- "{{ openshift_installer_dir }}/install-config.yaml"
- "{{ openshift_installer_dir }}/install-config.yaml.bkp"

- name: cleanup previous install artifacts
file:
state: absent
path: "{{ item }}"
with_items:
- "{{ openshift_installer_dir }}/.openshift_install_state.json"
- "{{ openshift_installer_dir }}/.openshift_install.log"
- "{{ openshift_installer_dir }}/metadata.json"
- "{{ openshift_installer_dir }}/bootstrap.ign"
- "{{ openshift_installer_dir }}/master.ign"
- "{{ openshift_installer_dir }}/worker.ign"
- "{{ openshift_installer_dir }}/auth"

- name: create installation manifests
shell: >
{{ openshift_installer_dir }}/openshift-install create manifests
--dir={{ openshift_installer_dir }}

- name: create cache disk machine configs
template:
src: 98-cache-disk.yaml.j2
dest: "{{ openshift_installer_dir }}/openshift/98-cache-disk.yaml"
when: "openshift_installer_cache_disk != ''"

- name: create ignition files
shell: >
rm -f {{ openshift_installer_dir }}/.openshift_install_state.json &&
rm -f {{ openshift_installer_dir }}/.openshift_install.log &&
rm -f {{ openshift_installer_dir }}/metadata.json &&
rm -f {{ openshift_installer_dir }}/*.ign &&
rm -rf {{ openshift_installer_dir }}/auth &&
{{ openshift_installer_dir }}/openshift-install create ignition-configs
--dir={{ openshift_installer_dir }}
57 changes: 57 additions & 0 deletions app/roles/openshift-installer/templates/98-cache-disk.yaml.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
apiVersion: machineconfiguration.openshift.io/v1
kind: MachineConfig
metadata:
labels:
machineconfiguration.openshift.io/role: master
name: 98-cache-disk
spec:
config:
ignition:
version: 3.1.0
storage:
disks:
- device: /dev/{{ openshift_installer_cache_disk }}
Copy link
Member

Choose a reason for hiding this comment

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

I would intuitively set openshift_installer_cache_disk to /dev/sdb (or whatever disk) rather than sdb - so the full path to the disk...
That's why I wouldn't prepend /dev in this template

Copy link
Member Author

Choose a reason for hiding this comment

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

I almost did that, but there is another config option that asks for the disk to install the OS on. That option doesn't require a /dev prefix because the CoreOS kernel args don't allow the /dev prefix.

So I wanted to stay consistent with that.

wipeTable: True
partitions:
- sizeMiB: 50000
Copy link
Member

Choose a reason for hiding this comment

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

Idea: use 50% of the disk size (gathered by ansible facts in ansible_devices) rather than 50GB statically?
What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ansible doesn't have any facts about the host at this time. This file is created before the host is created or powered on. I don't think there is a clean way to do that with ipmi/ilo either.

startMiB: 0
label: var-lib-kubelet
number: 1
wipePartitionEntry: True
- sizeMiB: 0
startMiB: 0
label: var-lib-containers
number: 2
wipePartitionEntry: True
filesystems:
- path: /var/lib/kubelet
device: /dev/disk/by-partlabel/var-lib-kubelet
format: xfs
wipeFilesystem: True
- path: /var/lib/containers
device: /dev/disk/by-partlabel/var-lib-containers
format: xfs
wipeFilesystem: True
systemd:
units:
- name: var-lib-kubelet.mount
enabled: true
contents: |
[Unit]
Before=local-fs.target
[Mount]
Where=/var/lib/kubelet
What=/dev/disk/by-partlabel/var-lib-kubelet
[Install]
WantedBy=local-fs.target
- name: var-lib-containers.mount
enabled: true
contents: |
[Unit]
Before=local-fs.target
[Mount]
Where=/var/lib/containers
What=/dev/disk/by-partlabel/var-lib-containers
[Install]
WantedBy=local-fs.target