Add basic cloudapi support #150

bcl · 2024-11-13T22:11:37Z

This adds support for cloudapi in the existing version of osbuild-composer

cloudapi openapi version shown as part of composer-cli status show
Start a cloudapi compose using a local blueprint.toml file
Start a cloudapi compose with local blueprint and upload.toml
Add support for --wait to cloudapi compose start and to composer-cli wait command

github-actions · 2024-12-14T04:27:34Z

This PR is stale because it has been open 30 days with no activity. Remove "Stale" label or comment or this will be closed in 7 days.

bcl · 2024-12-17T17:04:37Z

Needs to copy arch from the blueprint, if set, to the cloudapi request instead of using the host arch.

github-actions · 2025-01-17T04:26:06Z

This PR is stale because it has been open 30 days with no activity. Remove "Stale" label or comment or this will be closed in 7 days.

- PostRaw and PostJSON functions - GetJSON function - Exists function with test support - ErrorToString function to parse a cloudapi error json response into a string - Mock cloud http client for testing - Tests Related: RHEL-60148

Related: RHEL-60148

This allows you to build weldr-client with a custom version, eg. for development so that the NEVRA will always be greater than the last build. Use it like this: BASEVER=$(grep ^VERSION Makefile | awk '{ print $3 }') VERSION=$BASEVER.$(date +%Y%m%d%H%M) make build-in-podman

This will check to see if the cloudapi socket is available, and if so it will show the Cloud API version number and name of the service. Related: RHEL-60148

Retrieve the cloud API's openapi info. Related: RHEL-60148

This function uses the cloud API to start a compose using a blueprint that is included in the compose request instead of one that has been previously uploaded. Related: RHEL-60124

When a local file is used in place of the blueprint name composer-cli will use the cloud API to upload the blueprint and start the compose. Currently this expects the osbuild-composer server to have the 'localsave' option enabled in its osbuild-composer.service file. Resolves: RHEL-60124

The upload options are passed via the cloud API's image request ImageOptions: https://github.com/osbuild/osbuild-composer/blob/main/internal/cloudapi/v2/openapi.v2.yml#L795 the cloud client code does not do any validation of the format or content, that is all handled by the server which will return an error describing what went wrong if there is a problem. Related: RHEL-60124

The upload format for the cloud API server is different from that used by the weldr API. It is described in the cloud API's openapi schema document in the UploadOptions section: https://github.com/osbuild/osbuild-composer/blob/main/internal/cloudapi/v2/openapi.v2.yml#L1015 Related: RHEL-60124

The ExecutionError already prints 'ERROR' so there is no need for it in the strings passed to it. Related: RHEL-60148

Related: RHEL-60123

Related: RHEL-60148

Related: RHEL-60123

The composes built with the cloud API are not stored in the weldr API store so the UUID from the user may belong to one or the other. Check cloud API first, then weldr API and return the weldr API error if it is not found in either place. Note that the result printed is different. Cloud API uses 'success' and 'failure' and WELDR API uses 'FINISHED' and 'FAILED'. Related: RHEL-60124 Related: RHEL-60134

Related: RHEL-60123

Related: RHEL-60148

bcl · 2025-02-07T19:15:21Z

Added tests for ComposeInfo and ComposeWait

supakeen

Partial review, will continue later.

supakeen · 2025-02-08T08:06:42Z

cloud/common.go

+// used to query the server.
+func NewClient(ctx context.Context, socket HTTPClient, socketPath string) Client {
+	// TODO
+	// - check for valid server path


I'll assume this is about the socketPath, what kind of validation do you want to do?

Good question. There's a similar comment in the weldr client version of this. I think it may be old. IIRC I was going to check to make sure the socket existed here, but it ends up being easier to use the Exists() and checkSocketError functions since the commands first try cloud and then fall back to weldr if it isn't available. So I'm pretty sure these TODOs can now go away.

cloud/common.go

supakeen · 2025-02-08T08:18:19Z

cloud/common.go

+	if route[0] == '/' {
+		route = route[1:]
+	}
+	return fmt.Sprintf("%s://%s/%s", c.protocol, c.host, route)


I prefer to use net/url's URL.String() here.

That would work too, but I prefer to leave it for now. The protocol/host code needs a refactor in a later commit -- it was never hooked up to the cmdline and is always http and localhost. I think we're always going to be using domain sockets for this so they can go away at some point.

supakeen · 2025-02-08T08:21:14Z

cloud/common.go

+
+// IsStringInSlice returns true if the string is present, false if not
+// slice must be sorted
+func IsStringInSlice(slice []string, s string) bool {


Can this be slices.Contains instead or was that introduced later than our minimum version?

I change that in a followup commit that refactors some common things between cloud and weldr into a common package.

supakeen · 2025-02-08T08:22:15Z

cloud/common.go

+}
+
+// GetContentFilename returns the filename from a content disposition header
+func GetContentFilename(header string) (string, error) {


This can be mime.ParseMediaType; you'll get out a map that contains "filename" as the key.

Good idea. I'll do that in the followup PR (I move both copies of this into a common location)

supakeen · 2025-02-08T08:24:02Z

cloud/common.go

+}
+
+// AppendQuery adds the query string to the current url using ? for the first and & for subsequent ones
+func AppendQuery(url, query string) string {


This can also be done through net/url.

True. See comment on other url handling.

achilleas-k · 2025-02-12T16:36:39Z

cloud/common.go

+	host       string // defaults to localhost
+	socketPath string
+	rawFunc    func(string, string, int, []byte) // Pass the raw json data to a user function
+	Test       bool                              // Used to fake the presense of the socket for testing


Normally we would make Client an interface with an additional test implementation, but I can see why that would be a hassle. Still, it might be nicer if this was a private property that could be set with a separate constructor, NewTestClient(). Kinda feels strange to have it there to set freely. But I wont block on it.

achilleas-k · 2025-02-12T16:52:05Z

cmd/composer-cli/compose/start.go

+	// Is the blueprint a local file? If so, try to use the cloud API for the compose
+	f, err := os.Open(args[0])
+	if err == nil {
+		defer f.Close()
+
+		if !root.Cloud.Exists() {
+			return root.ExecutionError(cmd, "Using a local blueprint requires server support. Check to make sure that the cloudapi socket is enabled.")


This is probably not a real issue because of extension conventions, but it is conceivable that someone might do:

echo 'name = "myblueprint"' > myblueprint composer-cli blueprints push myblueprint composer-cli compose start myblueprint qcow2

and get this error.

achilleas-k · 2025-02-12T16:58:49Z

cmd/composer-cli/compose/start.go

+		if err != nil {
+			return root.ExecutionError(cmd, "Error starting cloud API compose: %s", err)
+		}
+	} else {


Might be better to check if the error is os.ErrNotExist than just assuming it's a blueprint name when os.Open() fails. It could be a permission error, for example, in which case the real error is never shown. Instead of getting a Permission denied, the user would get some error about the blueprint not existing from osbuild-composer.

bcl mentioned this pull request Nov 13, 2024

Adding cloudapi support to composer-cli (COMPOSER-2131) #134

Closed

github-actions bot added the Stale label Dec 14, 2024

bcl removed the Stale label Dec 17, 2024

bcl marked this pull request as draft December 17, 2024 17:04

github-actions bot added the Stale label Jan 17, 2025

bcl removed the Stale label Jan 22, 2025

bcl force-pushed the main-cloudapi-part1 branch from 7341469 to 1a4a012 Compare February 3, 2025 23:58

bcl marked this pull request as ready for review February 6, 2025 01:01

bcl force-pushed the main-cloudapi-part1 branch from 1a4a012 to f3f3464 Compare February 6, 2025 01:02

supakeen self-requested a review February 6, 2025 07:43

achilleas-k self-requested a review February 6, 2025 09:54

bcl added 10 commits February 6, 2025 09:42

cloud: Add basic Client functions for cloud api

bcff5a7

- PostRaw and PostJSON functions - GetJSON function - Exists function with test support - ErrorToString function to parse a cloudapi error json response into a string - Mock cloud http client for testing - Tests Related: RHEL-60148

cmd: Add cloudapi client

2e581d7

Related: RHEL-60148

status: Add cloudapi status to show command

9c71b3f

This will check to see if the cloudapi socket is available, and if so it will show the Cloud API version number and name of the service. Related: RHEL-60148

cloud: Add ServerStatus function

e96ddc4

Retrieve the cloud API's openapi info. Related: RHEL-60148

cloud: Add StartCompose function

55e65ff

This function uses the cloud API to start a compose using a blueprint that is included in the compose request instead of one that has been previously uploaded. Related: RHEL-60124

compose: Remove redundant 'Error' from error strings in start

fe4f7d6

The ExecutionError already prints 'ERROR' so there is no need for it in the strings passed to it. Related: RHEL-60148

bcl force-pushed the main-cloudapi-part1 branch from f3f3464 to a83140d Compare February 6, 2025 17:55

bcl added 5 commits February 7, 2025 11:13

cloud: Add ComposeInfo function

dba77dc

Related: RHEL-60123

cloud: Add test for ComposeInfo

07c859e

Related: RHEL-60148

cloud: Add ComposeWait function

29d7cb7

Related: RHEL-60123

compose: Add cloud API --wait to start command

f55a639

Related: RHEL-60123

cloud: Add tests for ComposeWait function

06dc9d6

Related: RHEL-60148

bcl force-pushed the main-cloudapi-part1 branch from a83140d to 06dc9d6 Compare February 7, 2025 19:14

supakeen reviewed Feb 8, 2025

View reviewed changes

achilleas-k reviewed Feb 12, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add basic cloudapi support #150

Add basic cloudapi support #150

bcl commented Nov 13, 2024

github-actions bot commented Dec 14, 2024

bcl commented Dec 17, 2024

github-actions bot commented Jan 17, 2025

bcl commented Feb 7, 2025

supakeen left a comment

supakeen Feb 8, 2025

bcl Feb 10, 2025

supakeen Feb 8, 2025

bcl Feb 10, 2025

supakeen Feb 8, 2025

bcl Feb 10, 2025

supakeen Feb 8, 2025

bcl Feb 10, 2025

supakeen Feb 8, 2025

bcl Feb 10, 2025

achilleas-k Feb 12, 2025

achilleas-k Feb 12, 2025

achilleas-k Feb 12, 2025

Add basic cloudapi support #150

Are you sure you want to change the base?

Add basic cloudapi support #150

Conversation

bcl commented Nov 13, 2024

github-actions bot commented Dec 14, 2024

bcl commented Dec 17, 2024

github-actions bot commented Jan 17, 2025

bcl commented Feb 7, 2025

supakeen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment