Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ require (
github.com/gobwas/glob v0.2.3
github.com/google/go-cmp v0.7.0
github.com/google/uuid v1.6.0
github.com/gophercloud/gophercloud/v2 v2.8.0
github.com/hashicorp/go-retryablehttp v0.7.8
github.com/hashicorp/go-version v1.7.0
github.com/kolo/xmlrpc v0.0.0-20220921171641-a4b6fa1dd06b
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,8 @@ github.com/googleapis/enterprise-certificate-proxy v0.3.6 h1:GW/XbdyBFQ8Qe+YAmFU
github.com/googleapis/enterprise-certificate-proxy v0.3.6/go.mod h1:MkHOF77EYAE7qfSuSS9PU6g4Nt4e11cnsDUowfwewLA=
github.com/googleapis/gax-go/v2 v2.15.0 h1:SyjDc1mGgZU5LncH8gimWo9lW1DtIfPibOG81vgd/bo=
github.com/googleapis/gax-go/v2 v2.15.0/go.mod h1:zVVkkxAQHa1RQpg9z2AUCMnKhi0Qld9rcmyfL1OZhoc=
github.com/gophercloud/gophercloud/v2 v2.8.0 h1:of2+8tT6+FbEYHfYC8GBu8TXJNsXYSNm9KuvpX7Neqo=
github.com/gophercloud/gophercloud/v2 v2.8.0/go.mod h1:Ki/ILhYZr/5EPebrPL9Ej+tUg4lqx71/YH2JWVeU+Qk=
github.com/gorilla/mux v1.8.1 h1:TuBL49tXwgrFYWhqrNgrUNEY92u81SPhu7sTdzQEiWY=
github.com/gorilla/mux v1.8.1/go.mod h1:AKf9I4AEqPTmMytcMc0KkNouC66V3BtZ4qD5fmWSiMQ=
github.com/grpc-ecosystem/grpc-gateway/v2 v2.25.1 h1:VNqngBF40hVlDloBruUehVYC3ArSgIyScOAyMRqBxRg=
Expand Down
93 changes: 93 additions & 0 deletions pkg/cloud/openstack/openstack.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
package openstack

import (
"context"
"fmt"
"io"
"os"

"github.com/osbuild/images/pkg/cloud"

"github.com/gophercloud/gophercloud/v2"
ostack "github.com/gophercloud/gophercloud/v2/openstack"
"github.com/gophercloud/gophercloud/v2/openstack/image/v2/imagedata"
"github.com/gophercloud/gophercloud/v2/openstack/image/v2/images"
)

var _ = cloud.Uploader(&openstackUploader{})

type openstackUploader struct {
image string
diskFormat string
containerFormat string
}

type UploaderOptions struct {
DiskFormat string
ContainerFormat string
}

func NewUploader(image string, opts *UploaderOptions) (cloud.Uploader, error) {
return &openstackUploader{
image: image,
diskFormat: opts.DiskFormat,
containerFormat: opts.ContainerFormat,
}, nil
}

func (ou *openstackUploader) Check(status io.Writer) error {
return nil
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be nice to do some of the checks we do in UploadAndRegister here already. Not a blocker though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To be honest, I am confused about the purpose of the Check method. My expectation was that if the check fails, we don't even try to upload. Which doesn't seem to be true. I return errors from my Check and UploadAndRegister gets called anyway.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, interesting. Then I must misremember how this works, its fine for now, we can always refactor it. The idea was that we would run "Check()" before we do an expensive upload and then discover that some permissions to move the image into the right place are missing (iirc that can happen on AWS). But if you say its not working for you we will investigate. Its easy enough to move some of the checks here in a followup.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, adding the checks later should be easy. Check this out.

func (ou *openstackUploader) Check(status io.Writer) error {
	return errors.New("FAILED UPLOAD")
}

The upload starts happening

[jkadlcik@hive image-builder]$ ./image-builder upload --to openstack ~/git/copr-image-builder/output/image/disk.raw --arch x86_64 --openstack-image frostyx_copr_hv_x86_64_01_prod_06740723_20250913_1418098
Uploading to OpenStack...
0 B / 10.00 GiB [________________________________________________________________________________________________________________________________________________________________________________________________________________] 0.00% ? p/s

And it's not a glitch because the "Uploading to OpenStack..." is there also. So we are definitely inside of the UploadAndRegister function.

}

func (ou *openstackUploader) UploadAndRegister(r io.Reader, uploadSize uint64, status io.Writer) (err error) {
fmt.Fprintf(status, "Uploading to OpenStack...\n")

opts, err := ostack.AuthOptionsFromEnv()
if err != nil {
return fmt.Errorf("Failed to read OpenStack ENV variables. Please source the OpenStack RC file: %w", err)
}

// This is needed otherwise we get the following error when authenticating:
// You must provide exactly one of DomainID or DomainName to
// authenticate by Username
// Even with an RC file that works perfectly fine with `openstack token issue`
Copy link

Choose a reason for hiding this comment

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

This seems to be related to this issue: gophercloud/gophercloud#3440
TL;DR It currently only uses the OS_DOMAIN_NAME env instead of OS_PROJECT_DOMAIN_NAME and OS_USER_DOMAIN_NAME env variables.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I linked the issue from within the code comment.

// See https://github.com/gophercloud/gophercloud/issues/3440
// See https://github.com/gophercloud/gophercloud/issues/3240
if opts.DomainName == "" {
opts.DomainName = os.Getenv("OS_USER_DOMAIN_NAME")
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we double check that os.Getenv("OS_USER_DOMAIN_NAME") != "" ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think we should. If it is "" then it will fail with

error: Failed to authenticate to OpenStack: You must provide exactly one of DomainID or DomainName to authenticate by Username

which is not ideal but it is better than failing because it is empty even though the upload might maybe work without that? I am not sure what happens on other OpenStack instances.

But if you prefer to have the check, I can definitely add it.

}

ctx := context.Background()
provider, err := ostack.AuthenticatedClient(ctx, opts)
if err != nil {
return fmt.Errorf("Failed to authenticate to OpenStack: %w", err)
}

client, err := ostack.NewImageV2(provider, gophercloud.EndpointOpts{
Region: os.Getenv("OS_REGION_NAME"),
})
if err != nil {
return fmt.Errorf("Failed to initialize the client: %w", err)
}

createOpts := images.CreateOpts{
Name: ou.image,
DiskFormat: ou.diskFormat,
ContainerFormat: ou.containerFormat,
}
img, err := images.Create(ctx, client, createOpts).Extract()
if err != nil {
return fmt.Errorf("Failed to create the image metadata: %w", err)
}

err = imagedata.Upload(ctx, client, img.ID, r).ExtractErr()
if err != nil {
return fmt.Errorf("Failed to upload the image: %w", err)
}

// This would glitch the progressbar, but once it gets fixed, we would
// like to print this message
// fmt.Printf("Created image: %s (ID: %s)\n", img.Name, img.ID)

return nil
}
Loading