-
Notifications
You must be signed in to change notification settings - Fork 991
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
Fixes #27604 - added Fedora|Redhat CoreOS and IM #8042
Conversation
Issues: #27604 |
db/seeds.d/100-installation_media.rb
Outdated
{ | ||
:name => "CentOS 7 mirror", | ||
:os_family => "Redhat", | ||
:path => "http://mirror.centos.org/centos/$major/os/$arch" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/TrailingCommaInHashLiteral: Put a comma after the last item of a multiline hash.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not resolved. We are enforcing comma here.
Hi, just wanted to let you know that i'm testing this patch actually our ci builds actually our foreman container image in version 2.1.3 on top this pull request gets patched into the image (slighty modificated [commented some apipie stuff out because foreman 2.1.3 does not seem to support it so it breaks while it starts] but i need still some time and on the weekend i mostly enjoy my weekend so mostly there i do no computer relevant stuff :) best regards |
For completeness, this came from https://community.theforeman.org/t/rhcos-fhcos-provisioned-for-openshift/20704. https://community.theforeman.org/t/rhcos-installation-with-foreman/18702 is another user who asked for this support. |
I've fixed the rendering problem and tests. The diff is small: diff --git a/lib/foreman/renderer/scope/variables/base.rb b/lib/foreman/renderer/scope/variables/base.rb
index 25e7ab9e9..c2342f975 100644
--- a/lib/foreman/renderer/scope/variables/base.rb
+++ b/lib/foreman/renderer/scope/variables/base.rb
@@ -36,7 +36,7 @@ module Foreman
@template_url = params['url']
end
- %w(coreos aif memdisk ZTP).each do |name|
+ %w(coreos fcos rhcos aif memdisk ZTP).each do |name|
define_method("#{name}_attributes") do
@mediapath = mediumpath(@medium_provider) if medium
end That will fix it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i tested it with this change now i was able to create a rhcos vm in foreman but it was not able to find its kernel
app/models/operatingsystems/rhcos.rb
Outdated
# http://mirror.openshift.com/pub/openshift-v4/x86_64/dependencies/rhcos/4.5/4.5.6/rhcos-installer-initramfs.x86_64.img | ||
# | ||
PXEFILES = { | ||
kernel: 'rhcos-installer-kernel-$arch', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a bug! I am rebasing this PR to fix this, good catch. Here is the diff:
diff --git a/app/models/operatingsystems/fcos.rb b/app/models/operatingsystems/fcos.rb
index 1d894c54d..8286f1b34 100644
--- a/app/models/operatingsystems/fcos.rb
+++ b/app/models/operatingsystems/fcos.rb
@@ -13,6 +13,10 @@ class Fcos < Operatingsystem
'fcos'
end
+ def bootfile(medium_provider, type)
+ medium_provider.interpolate_vars(super).to_s
+ end
+
def pxedir(medium_provider = nil)
medium_provider.interpolate_vars('prod/streams/$release/builds/$major.$minor/$arch').to_s
end
diff --git a/app/models/operatingsystems/rhcos.rb b/app/models/operatingsystems/rhcos.rb
index 7a2ceefaf..7fa7c7aab 100644
--- a/app/models/operatingsystems/rhcos.rb
+++ b/app/models/operatingsystems/rhcos.rb
@@ -13,6 +13,10 @@ class Rhcos < Operatingsystem
'rhcos'
end
+ def bootfile(medium_provider, type)
+ medium_provider.interpolate_vars(super).to_s
+ end
+
def pxedir(medium_provider = nil)
medium_provider.interpolate_vars('pub/openshift-v$major/$arch/dependencies/rhcos/$major.$minor/$major.$minor.$release').to_s
end
Thanks for testing!
Great, next up is adding the 3rd boot file for download (the image). But let's wait until this is merged. |
@ezr-ondrej I see you assigned yourself, if you don't mind reviewing all three PRs I am planning in this regard. If @Elyytscha can help with testing that would be fantastic! The other one is #8059 and I am planning one more once these are merged. Also quite small. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://builds.coreos.fedoraproject.org
instead of
http://builds.coreos.fedoraproject.org
and also the fcos mirror will work:
the icon problem seems to be existent for rhcos/fcos the url it wants to get the icon from is:
/images/icons16x16/rhcos.png
/images/icons16x16/fcos.png
maybe could be changed to the normal rhel // fedora icons?
'major' => '32', | ||
'minor' => '20200907.3.0', | ||
'release_name' => 'stable', | ||
'kernel' => 'http://builds.coreos.fedoraproject.org/prod/streams/stable/builds/32.20200907.3.0/x86_64/fedora-coreos-32.20200907.3.0-live-kernel-x86_64', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be also https
'minor' => '20200907.3.0', | ||
'release_name' => 'stable', | ||
'kernel' => 'http://builds.coreos.fedoraproject.org/prod/streams/stable/builds/32.20200907.3.0/x86_64/fedora-coreos-32.20200907.3.0-live-kernel-x86_64', | ||
'initrd' => 'http://builds.coreos.fedoraproject.org/prod/streams/stable/builds/32.20200907.3.0/x86_64/fedora-coreos-32.20200907.3.0-live-kernel-x86_64', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be also https
db/seeds.d/100-installation_media.rb
Outdated
{ | ||
:name => "Fedora CoreOS mirror", | ||
:os_family => "Fcos", | ||
:path => "http://builds.coreos.fedoraproject.org", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be https instead, with https i got it working, with http not
app/models/operatingsystems/rhcos.rb
Outdated
# http://mirror.openshift.com/pub/openshift-v4/x86_64/dependencies/rhcos/4.5/4.5.6/rhcos-installer-initramfs.x86_64.img | ||
# | ||
PXEFILES = { | ||
kernel: 'rhcos-installer-kernel-$arch', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
broken with openshift 4.6.. because of a rename..
i think its good that it was not merged yet!
Fixed https, icons and also renamed installer to live for 4.6 release. Thanks for testing! @ezr-ondrej |
Retriggering tests, 3 failed. |
for sure i can, its just not perfect and not usable for a general fcos/rhcos provisioning i think, but maybe it helps
ah, and i forgot, the last ugly thing is our openshift ansible has to cancel the foreman host build after the pxe boot happend, because i never found out how to do a working provisioning callback to foreman after the openshift host was installed why we did it this way: when you want to install openshift via the general UPI method, you will notice that you need a tftp, http server, foreman had all this already up and working so we just used foremans infrastructure for this and integrated the hosts as much as possible into our foreman i hope this makes sense somehow and i hope it will help a little bit |
I amended test fixes both for core and katello. @Elyytscha can you take your last comment and convert it into a PR? We will appreciate the contribution, let's discuss the details there. Parametrization is wanted, but you do not need to parametrize everything - just enough so others can use it too. |
I believe we would need to download the rootfs ourselves with the pxe files and change the initrd to include it, instead host params for the version, but apart of that I think it would be nice contribution already. |
[test katello] |
[test katello] not sure what's wrong with katello |
Aaron Patterson released new version of Psych YAML parser 4.0 which is a major change - by default it safe-loads all YAML. It's good for all of us: ruby/psych#487 anyway Katello codebase has an open ended depednency and that's why it fails:
I am writing a PSA to the community and I will file a PR for Katello team to unblock the CI: https://community.theforeman.org/t/psa-rubygem-psych-4-0-is-out-and-its-a-breaking-change/23604 |
Here is a patch for Katello plugin: Katello/katello#9365 |
Okay solved the problem with psych 4.0 YAML parser in Katello, rebasing. |
It still doesn't work... different reason now though. |
[test katello] |
[test katello] it is still pulling in psych 4.0 :( |
[test katello] one more time to a random reader: if you are reading this and considering getting into software engineering for having fun, lemme tell you: ITS NOT FUN! |
[test katello] |
1 similar comment
[test katello] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice addition! Thanks @lzap ! 👍
I'd love to see follow-ups so we can provision CoreOSes, so I hope you won't follow on your own advice and become carpenter xD
Thanks, @Elyytscha can you submit template changes? What else is left to get this working then? I would love to demo this if we got it working. |
@lzap i think whats needed now is:
with this 4 points, foreman would be able to install openshift container platform and openshift origin completly native i wrote you an email so you can reach me faster for details if you need help or if i need help with the pullrequest for the pxe template. |
Thanks for the writeup. I am currently busy with other things so this is not on my short-term agenda, feel free to file PRs. I did not receive any email tho, its lzap_at_redhat_com if you meant me. I am here to help with reviews. |
Hi guys, trying implement something similar as @Elyytscha. Could you suggest or reccomend how to do "step 2" after FCOS successful install. I would like to have it dynamically, not manual cancel.
Thanks in advice. :) |
This patch adds two new OSes:
Fedora CoreOS uses quite lengthy version
32.20200907.3.0
whichunfortunately cannot be stored into major or minor version field which
is by default validated to be numeric. Thing is, after validation both
major and minor are converted to strings and stored in database as
strings. Operating system versions are never compared as numbers (they
are fetched back as strings), therefore those numric validations were
removed - there is no reason to enforce them to be numbers.
We are not doing a good job explaining to our users what exactly should
be entered into major, minor and release_name fields. As part of the
patch, three new
*_help
methods are added to the operating systemwhere a user-facing help string can be displayed for major, minor and
release_name fields with closer explanation.
Warning: This is not end-to-end FedoraCoreOS provisioning, but it's probably the first in the series to enable the workflow.