support replication of images across regions for IBMCloud and PowerVS by Prashanth684 · Pull Request #2361 · coreos/coreos-assembler

Prashanth684 · 2021-08-11T23:03:24Z

This PR includes the following changes to support replication of IBMCloud images across regions:

Add a copy-object command to ore/ibmcloud to copy an image from a bucket in one region to a bucket in another region
Change schema of IBMCloud/PowerVS to an array rather than object to support multiple entries for the different buckets and regions
Add replication functionality to cosalib/ibmcloud.py which can be leveraged in the pipeline.
Add IBMCloud/PowerVS uploaded images to release.json

clnperez · 2021-08-11T23:24:52Z

Looks good to me from a Power VS POV.

Prashanth684 · 2021-08-19T15:50:39Z

@darkmuggle if you could take a look at the schema changes that would be great!

miabbott

I did a local container build of this PR on top of main and then used the result to build RHCOS artifacts (ibmcloud, powervs) successfully. Did make schema and make check too. (make check failed on the gangplank minio test, but that doesn't look related to this change)

Overall, this looks sane, but I don't have a way to test the replication bits easily.

Prashanth684 · 2021-08-24T23:38:47Z

I did a local container build of this PR on top of main and then used the result to build RHCOS artifacts (ibmcloud, powervs) successfully. Did make schema and make check too. (make check failed on the gangplank minio test, but that doesn't look related to this change)

Overall, this looks sane, but I don't have a way to test the replication bits easily.

thanks @miabbott for testing the schema ! yes the replication/upload requires an IBMCloud account. I did test the upload/replicate so i believe we are good there.

Prashanth684 · 2021-08-25T17:58:24Z

rebased. if this could be merged that would be great!

ravanelli

It looks sane to me. Would be nice to have a final word from @clnperez or even @mkumatag to double check the api in IBM side.

mkumatag · 2021-08-26T13:52:13Z

It looks sane to me. Would be nice to have a final word from @clnperez or even @mkumatag to double check the api in IBM side.

@Madhan-SWE can you please review this PR

dustymabe

Thanks for working on this. I've got a few questions and some comments..

Where are we planning to use this? It's my understanding that we currently can't get FCOS/RHCOS updated as a "stock image" in IBMCloud so we'd end up having our users uploading the image themselves anyway.
the schema stuff is a mess (not your fault). There are multiple copies of the schema (introduced in 9185dd0) and we need to figure out a coherent story there.
also, this type of change probably warrants a bump in schema version, which I don't know the mechanics of. Will try to bring someone else in to help advise.

dustymabe · 2021-08-26T21:07:40Z

+        args.region = ['au-syd', 'ca-tor', 'eu-de', 'eu-gb', 'jp-osa', 'jp-tok', 'us-east', 'us-south']
+        log.info(("default: replicating to all regions. If this is not "
+                 " desirable, use '--regions'"))


hmm. would be really nice not to hardcode stuff here. For AWS we created a ore aws list-regions command and used that to come up with the list of regions:

coreos-assembler/src/cosalib/aws.py

Lines 40 to 43 in e31bbb4

if not args.region:

args.region = subprocess.check_output([

'ore', 'aws', 'list-regions'

]).decode().strip().split()

Also, do all regions support powervs and ibmcloud artifacts? Might want to add some smarts there.

iiuc there is no API in the IBMCloud sdk that lists the regions. which is why i hardcoded it here. @mkumatag can you confirm this?

Also, as far as supported artifacts are concerned, any artifact can be uploaded to all regions, but PowerVS instances can only be created in certain regions which are the ones i've listed here.

iiuc there is no API in the IBMCloud sdk that lists the regions. which is why i hardcoded it here. @mkumatag can you confirm this?

not aware of any such API,!

There are a few things here to unpack. Assuming we have to hardcode let's

default to upload regions that are appropriate to the type (ibmcloud versus powervs)

link to documentation of where to find the list of regions that are supported by each platform (so the lists can be checked and updated later)

yes. i have already linked to the docs but i will confirm with the team and again update these regions based on the platform

After some clarification with the IBM team, it looks like PowerVS instances are supported in all the regions where cloud object storage buckets can be created. this is the same for IBMCloud as well. In addition to this - IBMCloud VMs can be created in additional data centers. so from a storage bucket perspective both images should be pushed to all regions and i have made the change to include all regions in the list common to both IBMCloud and PowerVS

dustymabe · 2021-08-26T21:41:02Z

also, for IBMCloud is it a good idea (or even possible) to delete the COS objects after the images are created? I think we do this in some other clouds.

Prashanth684 · 2021-08-26T22:16:26Z

Thanks for working on this. I've got a few questions and some comments..

Where are we planning to use this? It's my understanding that we currently can't get FCOS/RHCOS updated as a "stock image" in IBMCloud so we'd end up having our users uploading the image themselves anyway.

the schema stuff is a mess (not your fault). There are multiple copies of the schema (introduced in 9185dd0) and we need to figure out a coherent story there.

also, this type of change probably warrants a bump in schema version, which I don't know the mechanics of. Will try to bring someone else in to help advise.

The support is part of this enhancement: openshift/enhancements#736 mainly to support OCP IPI deployments. So the way it would work is - the rhcos pipeline uploads and replicates a powervs ova image to all regions and during install time, the terraform provider would consume that image and create a bootimage inside the PowerVS instance (inside a particular region) specified by the OCP install config. The reason the rhcos pipeline itself cannot create bootimages is because the bootimage needs to be part of a PowerVS "instance" which will be created by the user and supplied as a parameter in the install config.

Yes, i got a little confused with the schema stuff. Happy to clean up my implementation here, but i would like that any further consolidation be done in a separate PR and I will be happy to help! thanks for the review @dustymabe !

Prashanth684 · 2021-08-26T22:20:17Z

also, for IBMCloud is it a good idea (or even possible) to delete the COS objects after the images are created? I think we do this in some other clouds.

For the reason specified below as explained in the other comment, we cannot remove the cos objects because the bootimage is created during installation time.

So the way it would work is - the rhcos pipeline uploads and replicates a powervs ova image to all regions and
during install time, the terraform provider would consume that image and create a bootimage inside the PowerVS instance
(inside a particular region) specified by the OCP install config. The reason the rhcos pipeline itself cannot create bootimages >is because the bootimage needs to be part of a PowerVS "instance" which will be created by the user and supplied as a >parameter in the install config.

cgwalters · 2021-08-30T22:03:27Z

the rhcos pipeline uploads and replicates a powervs ova image to all regions

It'd be nice to not have RHCOS front-run FCOS in this respect; i.e. if we can get the creds to push FCOS in a similar way that'd be nice.

cgwalters

From my PoV this seems fine to merge; there are clearly things we could improve but they can be done as followup.

dustymabe · 2021-08-31T13:41:47Z

From my PoV this seems fine to merge; there are clearly things we could improve but they can be done as followup.

I think the biggest thing here is the schema stuff. It's sprawled out (not the fault of this PR), but we don't have to handle that here. But.. this PR does change some of the schema (it's not purely additive) and I think that requires a schema version bump (otherwise what's the point of versions?). I don't know if we've ever bumped the schema before so that'll be an exercise.

cgwalters · 2021-09-03T11:50:00Z

Actually, shouldn't we make a new ibmcloud toplevel section exactly like amis and gcp that tracks the replication state?

(Eventually, we may not need to do this at all - perhaps one thing we could do is actually record this in a "cosa build stream metadata" that gets just merged into our primary stream metadata, i.e. longer term we deprecate and drop the overlapping bits between meta.json and stream metadata)

dustymabe · 2021-09-15T18:49:23Z

            }
    # Aliyun/AWS specific additions
-    for meta_key, cloud, image_field in ("aliyun", "aliyun", "id"), ("amis", "aws", "hvm"):
+    for meta_key, cloud, image_field in ("aliyun", "aliyun", "id"), ("amis", "aws", "hvm"), ("ibmcloud", "ibmcloud", "image"), ("powervs", "powervs", "image"):


Do you mind showing me an example of what this looks like when run? Is it an image name or a URL? If it's a URL maybe we should be representing this a different way.

actually i realized that we need the bucket name to key off of also. i added that for IBMCloud/PowerVS. this is how the release meta looks:

"powervs": { "artifacts": { "ova": { "disk": { "location": "https://builds.coreos.fedoraproject.org/prod/streams/master/builds/410.84.202109171742-0/ppc64le/rhcos-410.84.202109171742-0-powervs.ppc64le.ova", "signature": "https://builds.coreos.fedoraproject.org/prod/streams/master/builds/410.84.202109171742-0/ppc64le/rhcos-410.84.202109171742-0-powervs.ppc64le.ova.sig", "sha256": "8b8ac96079aade6f813c3d4d33b6042920825b1e622b799d5e7a28fd515e1d87", "uncompressed-sha256": null } } }, "images": { "us-east": { "image": "rhcos-410-84-202109171742-0-powervs-ppc64le.ova", "bucket": "rhcos-powervs-images-us-east" }, "eu-de": { "image": "rhcos-410-84-202109171742-0-powervs-ppc64le.ova", "bucket": "rhcos-powervs-images-eu-de" }, "us-south": { "image": "rhcos-410-84-202109171742-0-powervs-ppc64le.ova", "bucket": "rhcos-powervs-images-us-south" } } },

Also this is before i added your suggestion below where the platform and the arch have swapped places.

@clnperez this is how the release metadata will look in the openshift installer. I take it that this information is sufficient for your needs ?

here's my complaint here. we're not actually creating an image are we? we're just uploading a file into object storage.

As such it would seem like we should be using the url key here instead of image. Maybe like is done here for azure: https://github.com/openshift/installer/blob/f6ece2a15c0d0fd57028be53df619cf48acefa11/data/data/rhcos-stream.json#L564

unfortunately power vs image import doesn't understand the url format, it understands only the bucket, region and object name but having that said, if we can include url also with less effort I'm okay(
may be helpful someone if they want to download this image and use them in their own bucket or so) but this format looks promising to me atm.

in order to replicate images from a bucket in a particular region to another bucket in a different region, introduce the copy-object command. this command can only copy to buckets in a particular region because for IBMCloud, the s3 client needs a specfic endpoint and thus needs to be associated with a region.

since we want to support replication across regional buckets, change schema to an array which contains information on image name, region, bucket and url

one of the requirements for the PowerVS platform is to have ova images replicated across collocation zones so clusters can be deployed in those zones. This change allows images to be replicated across specific regions and in buckets that are specified by prefixes so that it is helpful for the pipeline to create buckets and copy these images to those buckets.

Prashanth684 · 2021-09-21T19:13:16Z

made a few changes to the object schema:

included an object field in cloudartifacts to denote storage bucket objects
had to make the image field optional in cloudartifacts as IBMCloud and PowerVS artifacts would not have it.

An example of how the release meta looks:

"objects": {
            "us-east": {
              "object": "rhcos-410-84-202109211719-0-ppc64le-powervs.ova",
              "bucket": "rhcos-powervs-images-us-east",
              "url": "https://s3.us-east.cloud-object-storage.appdomain.cloud/rhcos-powervs-images-us-east/rhcos-410-84-202109211719-0-ppc64le-powervs.ova"
            },
            "us-south": {
              "object": "rhcos-410-84-202109211719-0-ppc64le-powervs.ova",
              "bucket": "rhcos-powervs-images-us-south",
              "url": "https://s3.us-south.cloud-object-storage.appdomain.cloud/rhcos-powervs-images-us-south/rhcos-410-84-202109211719-0-ppc64le-powervs.ova"
            },
            "eu-de": {
              "object": "rhcos-410-84-202109211719-0-ppc64le-powervs.ova",
              "bucket": "rhcos-powervs-images-eu-de",
              "url": "https://s3.eu-de.cloud-object-storage.appdomain.cloud/rhcos-powervs-images-eu-de/rhcos-410-84-202109211719-0-ppc64le-powervs.ova"
            }
          }

dustymabe · 2021-09-21T19:45:01Z

+	Image  string `json:"image,omitempty"`
+	Object string `json:"object,omitempty"`


@jlebon @cgwalters - I suggested @Prashanth684 add an object field here so they can identify things via bucket/object rather than re-use the image key here (which typically implies bootable image (like AMI)). Sound OK?

Hmm, feels like we should just get rid of the generic Cloudartifact, and have separate AzureArtifact, IbmCloudArtifact, and PowerVirtualServerArtifacts types. WDYT?

yeah, he previously had something like IbmCloudArtifact to represent both IBMCloud and PowerVS but I suggested he use CloudArtifact (at the time we hadn't proposed adding the new object field). I honestly think we don't really need that. Feels like re-using cloudArtifact here should be fine since it shares concepts with other clouds (unlike GCP, which has projects and image families and such).

@jlebon thoughts?

So let me make sure I understand:

Azure uses only Image and URL

IbmCloud uses Bucket, Object, Region, and Url

PowerVirtualServer uses Bucket and Object

?

I mean, it depends how "well-typed" we want to be. Those clearly could use separate types, and then validation would be that much stronger (which means e.g. we don't run the risk of pushing an azure key with fields which don't make sense). While in FCOS these details are mostly internal, because we have layers of metadata on top with very different schemas, in RHCOS the meta.json is only lightly transformed and shoved into the installer binary. Though it probably wouldn't really get far into CI if the required fields were actually wrong.

So overall, I'd lean towards tighter typing, but at the same time I don't think it's a pressing issue (and ideally eventually we stop hardcoding meta.json in the installer). So I wouldn't block on it either.

So let me make sure I understand:

* `Azure` uses only `Image` and `URL`

looks like it, though I will admit we don't use this type for FCOS so I don't know if the image: that's getting populated in the metadata for RHCOS is an actual bootable image or just the file name of the object in object storage.

* `IbmCloud` uses `Bucket`, `Object`, `Region`, and `Url`> * `PowerVirtualServer` uses `Bucket` and `Object`

IIUC both of these would populate Bucket, Object, Region, and Url. Obviously knowing enough about the structure of the cloud you can introspect URL from the other data. It's mostly there as a nice to have.

I mean, it depends how "well-typed" we want to be. Those clearly could use separate types, and then validation would be that much stronger (which means e.g. we don't run the risk of pushing an azure key with fields which don't make sense). While in FCOS these details are mostly internal, because we have layers of metadata on top with very different schemas, in RHCOS the meta.json is only lightly transformed and shoved into the installer binary. Though it probably wouldn't really get far into CI if the required fields were actually wrong.

So overall, I'd lean towards tighter typing, but at the same time I don't think it's a pressing issue (and ideally eventually we stop hardcoding meta.json in the installer). So I wouldn't block on it either.

I honestly think we need to take a step back and make sure all of our metadata variants make more sense.

For now I propose we just go forward with what we have here.

For now I propose we just go forward with what we have here.

Yup sure, WFM!

yes so IBMCloud artifacts and PowerVS artifacts would have the same fields as they both use the same cloud and similar object storage buckets.

…images we now have the ability to replicate IBMCloud/PowerVS images. add them to the release.json

dustymabe

Generally LGTM. Though still interested in @jlebon's discussion over in https://github.com/coreos/coreos-assembler/pull/2361/files#r713377123

Enable Power VS(Virtual Servers) as a platform. PowerVS is an extension of IBMCloud which supports deploying ppc64le VMs. Details here: openshift/enhancements#736 The hostname for the VMs are set through afterburn which reads the metadata provided through the config drive which is the primary mechanism through which ignition and hostname are provided. (coreos/afterburn#592) Cross referencing some other PRs for completeness: openshift/installer#5270 openshift/installer#5224 (closed, WIP to split into multiple PRs) ova images for PowerVS are already generated today: coreos/coreos-assembler#2200 coreos/coreos-assembler#2361

Prashanth684 commented Aug 11, 2021

View reviewed changes

Comment thread schema/generate-schema.sh

Prashanth684 commented Aug 11, 2021

View reviewed changes

Comment thread schema/cosa/schema_doc.go

Prashanth684 force-pushed the ibmcloud-replicate branch from 0a15e44 to cee7a66 Compare August 11, 2021 23:19

mkumatag suggested changes Aug 12, 2021

View reviewed changes

Comment thread mantle/cmd/ore/ibmcloud/copy-object.go

clnperez reviewed Aug 12, 2021

View reviewed changes

Comment thread src/cosalib/ibmcloud.py Outdated

miabbott requested a review from darkmuggle August 12, 2021 15:02

Prashanth684 force-pushed the ibmcloud-replicate branch from cee7a66 to 6d15e55 Compare August 12, 2021 22:31

ashcrow requested review from dustymabe and ravanelli and removed request for darkmuggle August 24, 2021 17:24

miabbott previously approved these changes Aug 24, 2021

View reviewed changes

Prashanth684 force-pushed the ibmcloud-replicate branch from 6d15e55 to 9061a67 Compare August 24, 2021 23:49

ravanelli reviewed Aug 26, 2021

View reviewed changes

dustymabe reviewed Aug 26, 2021

View reviewed changes

Prashanth684 force-pushed the ibmcloud-replicate branch 3 times, most recently from e97f40f to aa31b14 Compare August 27, 2021 20:42

cgwalters previously approved these changes Aug 30, 2021

View reviewed changes

Prashanth684 dismissed cgwalters’s stale review via ed77da6 September 7, 2021 15:16

dustymabe reviewed Sep 15, 2021

View reviewed changes

dustymabe reviewed Sep 17, 2021

View reviewed changes

Comment thread src/cmd-generate-release-meta

Comment thread src/cosalib/ibmcloud.py Outdated

Prashanth684 force-pushed the ibmcloud-replicate branch 4 times, most recently from ca1426b to 98375fb Compare September 17, 2021 19:07

dustymabe reviewed Sep 17, 2021

View reviewed changes

Comment thread src/cosalib/ibmcloud.py Outdated

Prashanth684 force-pushed the ibmcloud-replicate branch 2 times, most recently from 839658e to 3d36b0e Compare September 21, 2021 19:04

Prashanth684 force-pushed the ibmcloud-replicate branch from 3d36b0e to 109acd9 Compare September 21, 2021 19:06

Prashanth684 added 2 commits September 21, 2021 12:07

schema: change ibmcloud and powervs artifacts from object to array

4cb0425

since we want to support replication across regional buckets, change schema to an array which contains information on image name, region, bucket and url

Prashanth684 force-pushed the ibmcloud-replicate branch from 109acd9 to e2c9ff4 Compare September 21, 2021 19:07

Prashanth684 force-pushed the ibmcloud-replicate branch from e2c9ff4 to 625d0a7 Compare September 21, 2021 19:38

dustymabe reviewed Sep 21, 2021

View reviewed changes

Comment thread src/cmd-generate-release-meta Outdated

dustymabe reviewed Sep 21, 2021

View reviewed changes

Comment thread src/cmd-generate-release-meta Outdated

Prashanth684 force-pushed the ibmcloud-replicate branch from 625d0a7 to 65d10e0 Compare September 21, 2021 21:30

generate-release-meta: add support for uploaded IBMCloud and PowerVS …

bb30d84

…images we now have the ability to replicate IBMCloud/PowerVS images. add them to the release.json

Prashanth684 force-pushed the ibmcloud-replicate branch from 65d10e0 to bb30d84 Compare September 21, 2021 21:38

dustymabe approved these changes Sep 21, 2021

View reviewed changes

dustymabe merged commit f607717 into coreos:main Sep 22, 2021

This was referenced Oct 15, 2021

Add PowerVS platform openshift/machine-config-operator#2801

Merged

Add support for IBMCloud/PowerVS images coreos/stream-metadata-go#35

Merged

	if not args.region:
	args.region = subprocess.check_output([
	'ore', 'aws', 'list-regions'
	]).decode().strip().split()

		Image string `json:"image,omitempty"`
		Object string `json:"object,omitempty"`

Conversation

Prashanth684 commented Aug 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

clnperez commented Aug 11, 2021

Uh oh!

Uh oh!

Uh oh!

Prashanth684 commented Aug 19, 2021

Uh oh!

miabbott left a comment

Choose a reason for hiding this comment

Uh oh!

Prashanth684 commented Aug 24, 2021

Uh oh!

Prashanth684 commented Aug 25, 2021

Uh oh!

ravanelli left a comment

Choose a reason for hiding this comment

Uh oh!

mkumatag commented Aug 26, 2021

Uh oh!

dustymabe left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Prashanth684 Sep 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

dustymabe commented Aug 26, 2021

Uh oh!

Prashanth684 commented Aug 26, 2021

Uh oh!

Prashanth684 commented Aug 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cgwalters commented Aug 30, 2021

Uh oh!

cgwalters left a comment

Choose a reason for hiding this comment

Uh oh!

dustymabe commented Aug 31, 2021

Uh oh!

cgwalters commented Sep 3, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Prashanth684 Sep 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Prashanth684 commented Sep 21, 2021

Uh oh!

Prashanth684 commented Aug 11, 2021 •

edited

Loading

Prashanth684 Sep 7, 2021 •

edited

Loading

Prashanth684 commented Aug 26, 2021 •

edited

Loading

Prashanth684 Sep 17, 2021 •

edited

Loading

dustymabe Sep 21, 2021 •

edited

Loading