-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Refactor RHCOS fetch API, support fetching qemu from partial config #856
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,23 +1,16 @@ | ||
package rhcos | ||
|
||
import ( | ||
"context" | ||
|
||
"github.com/pkg/errors" | ||
) | ||
|
||
// AMI fetches the HVM AMI ID of the latest Red Hat CoreOS release. | ||
func AMI(ctx context.Context, channel, region string) (string, error) { | ||
meta, err := fetchLatestMetadata(ctx, channel) | ||
if err != nil { | ||
return "", errors.Wrap(err, "failed to fetch RHCOS metadata") | ||
} | ||
|
||
for _, ami := range meta.AMIs { | ||
func AMI(build Build, region string) (string, error) { | ||
for _, ami := range build.Meta.AMIs { | ||
if ami.Name == region { | ||
return ami.HVM, nil | ||
} | ||
} | ||
|
||
return "", errors.Errorf("no RHCOS AMIs found in %s", region) | ||
return "", errors.Errorf("RHCOS build %s does not have AMIs in region %s", build.Meta.OSTreeVersion, region) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,18 +1,15 @@ | ||
package rhcos | ||
|
||
import ( | ||
"context" | ||
"os" | ||
"fmt" | ||
|
||
"github.com/pkg/errors" | ||
) | ||
|
||
// QEMU fetches the URL of the latest Red Hat CoreOS release. | ||
func QEMU(ctx context.Context, channel string) (string, error) { | ||
meta, err := fetchLatestMetadata(ctx, channel) | ||
if err != nil { | ||
return "", errors.Wrap(err, "failed to fetch RHCOS metadata") | ||
func QEMU(build Build) string { | ||
qcowImage, ok := os.LookupEnv("OPENSHIFT_INSTALL_LIBVIRT_IMAGE") | ||
if ok { | ||
return qcowImage | ||
} | ||
|
||
return fmt.Sprintf("%s/%s/%s/%s", baseURL, channel, meta.OSTreeVersion, meta.Images.QEMU.Path), nil | ||
return fmt.Sprintf("%s/%s/%s/%s", build.BaseURL, build.Channel, build.Meta.OSTreeVersion, build.Meta.Images.QEMU.Path) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -76,9 +76,13 @@ func TFVars(cfg *types.InstallConfig, bootstrapIgn, masterIgn string) ([]byte, e | |
if cfg.Platform.AWS != nil { | ||
ctx, cancel := context.WithTimeout(context.TODO(), 30*time.Second) | ||
defer cancel() | ||
ami, err := rhcos.AMI(ctx, rhcos.DefaultChannel, cfg.Platform.AWS.Region) | ||
rhcosBuild, err := rhcos.FetchBuild(ctx, rhcos.DefaultChannel) | ||
if err != nil { | ||
return nil, errors.Wrap(err, "failed to determine default AMI") | ||
return nil, err | ||
} | ||
ami, err := rhcos.AMI(rhcosBuild, cfg.Platform.AWS.Region) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
config.AWS = aws.AWS{ | ||
|
@@ -97,13 +101,25 @@ func TFVars(cfg *types.InstallConfig, bootstrapIgn, masterIgn string) ([]byte, e | |
for i, ip := range cfg.Platform.Libvirt.MasterIPs { | ||
masterIPs[i] = ip.String() | ||
} | ||
|
||
// Do an on-demand fetch if the image isn't specified (e.g. user | ||
// is reusing an install config) | ||
qemuImage := cfg.Platform.Libvirt.DefaultMachinePlatform.Image | ||
if qemuImage == "" { | ||
rhcosBuild, err := rhcos.FetchBuild(context.TODO(), rhcos.DefaultChannel) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is too late (and the in-master AMI fetch above is also too late). We need these resolved by the time we generate the machine(-set) values we set up for the cluster API. #792 tries to straighten out dependencies to make this work. |
||
if err != nil { | ||
return nil, errors.Wrap(err, "failed to fetch QEMU image URL") | ||
} | ||
qemuImage = rhcos.QEMU(rhcosBuild) | ||
} | ||
|
||
config.Libvirt = libvirt.Libvirt{ | ||
URI: cfg.Platform.Libvirt.URI, | ||
Network: libvirt.Network{ | ||
IfName: cfg.Platform.Libvirt.Network.IfName, | ||
IPRange: cfg.Platform.Libvirt.Network.IPRange.String(), | ||
}, | ||
Image: cfg.Platform.Libvirt.DefaultMachinePlatform.Image, | ||
Image: qemuImage, | ||
MasterIPs: masterIPs, | ||
} | ||
if err := config.Libvirt.TFVars(config.Masters); err != nil { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,6 +13,8 @@ type MachinePool struct { | |
// Image is the URL to the OS image. | ||
// E.g. "http://aos-ostree.rhev-ci-vms.eng.rdu2.redhat.com/rhcos/images/cloud/latest/rhcos-qemu.qcow2.gz" | ||
Image string `json:"image"` | ||
// Metadata string describing the image | ||
ImageDescription string `json:"imageDescription,omitempty"` | ||
cgwalters marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
} | ||
|
||
// Set sets the values from `required` to `a`. | ||
|
@@ -30,4 +32,7 @@ func (l *MachinePool) Set(required *MachinePool) { | |
if required.Image != "" { | ||
l.Image = required.Image | ||
} | ||
if required.ImageDescription != "" { | ||
l.ImageDescription = required.ImageDescription | ||
} | ||
} |
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.
nit: The log message should describe the resource being fetched, but "URL" is just how we're fetching it. If you don't like "OS image", how about "QEMU image"? Or "QCOW2" (although I don't know if we actually require this to be QCOW2 or if libvirt can handle other formats too)?