Skip to content
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

pkg/rhcos: Default to the most-recent AMI #290

Merged
merged 2 commits into from
Sep 21, 2018

Conversation

wking
Copy link
Member

@wking wking commented Sep 20, 2018

We're pushing public AMIs since openshift/os#304. There's still no public analog to this, so I'm just scraping this from metadata on images available via the AWS API. The analogous AWS command line invocation is:

$ AWS_DEFAULT_REGION=us-east-1 aws ec2 describe-images --filter 'Name=name,Values=rhcos*' --query 'sort_by(Images, &CreationDate)[-1].ImageId' --output text

with a few extra filters thrown in. The full set of metadata on the most recent current image is:

$ AWS_DEFAULT_REGION=us-east-1 aws ec2 describe-images --filter 'Name=name,Values=rhcos*' --query 'sort_by(Images, &CreationDate)[-1]' --output json
{
    "VirtualizationType": "hvm",
    "Description": "Red Hat CoreOS 4.0.5846 (c9a6bb48b837b5bcfeb9bd427be9a18b5bd75b6c57cb289245f211ff98b2a740)",
    "Hypervisor": "xen",
    "EnaSupport": true,
    "SriovNetSupport": "simple",
    "ImageId": "ami-08a5792a684330602",
    "State": "available",
    "BlockDeviceMappings": [
        {
            "DeviceName": "/dev/xvda",
            "Ebs": {
                "Encrypted": false,
                "DeleteOnTermination": true,
                "VolumeType": "gp2",
                "VolumeSize": 8,
                "SnapshotId": "snap-00a45db4ad6173805"
            }
        },
        {
            "DeviceName": "/dev/xvdb",
            "VirtualName": "ephemeral0"
        }
    ],
    "Architecture": "x86_64",
    "ImageLocation": "531415883065/rhcos_dev_c9a6bb4-hvm",
    "RootDeviceType": "ebs",
    "OwnerId": "531415883065",
    "RootDeviceName": "/dev/xvda",
    "CreationDate": "2018-09-19T23:40:54.000Z",
    "Public": true,
    "ImageType": "machine",
    "Name": "rhcos_dev_c9a6bb4-hvm"
}

That doesn't include the "tested" information, so there's still no support for changing channels. We'll need to wait for a public analog of this, which is blocked on getting stable, production hosting for the release metadata.

I'd prefer to use JMESPath and server-side filtering in Go as well, to only return the latest matching AMI. But the AWS Go library doesn't seem to support server-side filtering at the moment (aws/aws-sdk-go#2156). Docs for the AWS Go APIs I'm using are here, here, here, here, here, and here.

The filters I'm adding here are similar to those we used for Container Linux before they were dropped in #233. I added a few more just to be conservative (e.g. we don't want to match a pending or failed image, so I require state to be available).

I haven't pushed the Context variables all the way up the stack yet, so there are some context.TODO() entries. The 30-second timeout keeps us from hanging excessively when the caller lacks AWS credentials; the error messages look like:

failed to init cluster: failed to parse test config: failed to determine default AMI: NoCredentialProviders: no valid providers in chain. Deprecated.
  For verbose messaging see aws.Config.CredentialsChainVerboseErrors

You can test this error condition by removing the explicit AMI values I've added to our fixtures in this commit and running:

$ AWS_PROFILE=does-not-exist go test ./installer/pkg/...

CC @ashcrow, @jupierce.

@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: wking

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 20, 2018
@openshift-ci-robot openshift-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 20, 2018
@wking
Copy link
Member Author

wking commented Sep 20, 2018

I've spun off some of the vendoring changes we need here into #291 (getting master back into a Glide-able state). There are some additional vendoring commits in this PR now as well, as take baby steps towards something that included a working glide get --strip-vendor github.com/aws/aws-sdk-go. Details in the commit messages.

pkg/rhcos/ami.go Outdated Show resolved Hide resolved
@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 20, 2018
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 20, 2018
@wking
Copy link
Member Author

wking commented Sep 21, 2018

Rebased around #287 with 801c21f -> 2343639.

We're pushing public AMIs since openshift/os@6dd20dc6 (jenkins: Make
RHCOS AMI Public, 2018-09-18, openshift/os#304).  There's still no
public analog to [1], so I'm just scraping this from metadata on
images available via the AWS API.  The analogous AWS command line
invocation is:

  $ AWS_DEFAULT_REGION=us-east-1 aws ec2 describe-images --filter 'Name=name,Values=rhcos*' --query 'sort_by(Images, &CreationDate)[-1].ImageId' --output text

with a few extra filters thrown in.  The full set of metadata on the
most recent current image is:

  $ AWS_DEFAULT_REGION=us-east-1 aws ec2 describe-images --filter 'Name=name,Values=rhcos*' --query 'sort_by(Images, &CreationDate)[-1]' --output json
  {
      "VirtualizationType": "hvm",
      "Description": "Red Hat CoreOS 4.0.5846 (c9a6bb48b837b5bcfeb9bd427be9a18b5bd75b6c57cb289245f211ff98b2a740)",
      "Hypervisor": "xen",
      "EnaSupport": true,
      "SriovNetSupport": "simple",
      "ImageId": "ami-08a5792a684330602",
      "State": "available",
      "BlockDeviceMappings": [
          {
              "DeviceName": "/dev/xvda",
              "Ebs": {
                  "Encrypted": false,
                  "DeleteOnTermination": true,
                  "VolumeType": "gp2",
                  "VolumeSize": 8,
                  "SnapshotId": "snap-00a45db4ad6173805"
              }
          },
          {
              "DeviceName": "/dev/xvdb",
              "VirtualName": "ephemeral0"
          }
      ],
      "Architecture": "x86_64",
      "ImageLocation": "531415883065/rhcos_dev_c9a6bb4-hvm",
      "RootDeviceType": "ebs",
      "OwnerId": "531415883065",
      "RootDeviceName": "/dev/xvda",
      "CreationDate": "2018-09-19T23:40:54.000Z",
      "Public": true,
      "ImageType": "machine",
      "Name": "rhcos_dev_c9a6bb4-hvm"
  }

That doesn't include the "tested" information, so there's still no
support for changing channels.  We'll need to wait for a public analog
of [1], which is blocked on getting stable, production hosting for the
release metadata.

I'd prefer to use JMESPath and server-side filtering in Go as well, to
only return the latest matching AMI.  But the AWS Go library doesn't
seem to support server-side filtering at the moment [2].  Docs for the
AWS Go APIs I'm using are in [3,4,5,6,7,8].

The filters I'm adding here are similar to those we used for Container
Linux before they were dropped in 702ee7b (*: Remove stale Container
Linux references, 2018-09-11, openshift#233).  I added a few more just to be
conservative (e.g. we don't want to match a pending or failed image,
so I require state to be available).

I haven't pushed the Context variables all the way up the stack yet,
so there are some context.TODO() entries.  The 30-second timeout keeps
us from hanging excessively when the caller lacks AWS credentials; the
error messages look like:

  failed to init cluster: failed to parse test config: failed to determine default AMI: NoCredentialProviders: no valid providers in chain. Deprecated.
    For verbose messaging see aws.Config.CredentialsChainVerboseErrors

You can test this error condition by removing the explicit AMI values
I've added to our fixtures in this commit and running:

  $ AWS_PROFILE=does-not-exist go test ./installer/pkg/...

[1]: http://aos-ostree.rhev-ci-vms.eng.rdu2.redhat.com/rhcos/images/aws-us-east-1-tested.json
[2]: aws/aws-sdk-go#2156
[3]: https://docs.aws.amazon.com/sdk-for-go/api/aws/session/#NewSessionWithOptions
[4]: https://docs.aws.amazon.com/sdk-for-go/api/aws/session/#Options
[5]: https://docs.aws.amazon.com/sdk-for-go/api/aws/session/#Must
[6]: https://docs.aws.amazon.com/sdk-for-go/api/service/ec2/#New
[7]: https://docs.aws.amazon.com/sdk-for-go/api/service/ec2/#EC2.DescribeImagesWithContext
[8]: https://docs.aws.amazon.com/sdk-for-go/api/service/ec2/#DescribeImagesInput
Generated with:

  $ glide get --strip-vendor github.com/aws/aws-sdk-go
  $ glide-vc --use-lock-file --no-tests --only-code
  $ bazel run //:gazelle

using:

  $ glide --version
  (cd $GOPATH/src/github.com/Masterminds/glide && git describe)
  v0.13.1-7-g3e13fd1
  $ (cd $GOPATH/src/github.com/sgotti/glide-vc && git describe)
  v0.1.0-2-g6ddf6ee
  $ bazel version
  Build label: 0.16.1- (@non-git)
  Build target: bazel-out/k8-opt/bin/src/main/java/com/google/devtools/build/lib/bazel/BazelServer_deploy.jar
  Build time: Mon Aug 13 16:42:29 2018 (1534178549)
  Build timestamp: 1534178549
  Build timestamp as int: 1534178549

The pkg/asset/manifests/BUILD.bazel bump catches us up with 161fac4
(asset/manifests: remove custom password generation, 2018-09-20,
@ashcrow
Copy link
Member

ashcrow commented Sep 21, 2018

@yuqi-zhang is looking at making the json public.

@wking
Copy link
Member Author

wking commented Sep 21, 2018

I'd prefer to use JMESPath and server-side filtering in Go as well...

Also, while I'd thought JMESPath was server-side on the CLI, it turns out that everybody is using JMESPath client-side.

@yuqi-zhang is looking at making the json public.

Will we be able to drop the AWS query then? Or do you think we'll still need it for regions not supported (yet?) by JSON files? Or to support more-current private releases?

@ashcrow
Copy link
Member

ashcrow commented Sep 21, 2018

@wking You should be able to read the file that @yuqi-zhang will be writing to an s3 bucket. It will denote the latest AMI. Unreleased AMI's won't be making it out to all the regions, but real releases will.

@wking wking deleted the floating-amis branch September 22, 2018 04:34
wking added a commit to wking/openshift-installer that referenced this pull request Oct 30, 2018
The OS folks have a new pipeline for publishing these AMIs, and
they're setting these new names.  Current output:

  $ curl -sL https://rhcos-release-browser-coreos.int.open.paas.redhat.com/storage/releases/maipo/47.33/meta.json | jq .amis
  [
    {
      "name": "us-east-1",
      "hvm": "ami-0fd0205c3c81d0412"
    }
  ]
  $ AWS_DEFAULT_REGION=us-east-1 aws ec2 describe-images --image-ids ami-0fd0205c3c81d0412 --output json
  {
     "Images": [
         {
             "VirtualizationType": "hvm",
             "Description": "Red Hat CoreOS \"maipo\" (OpenShift) 47.33",
             "Tags": [
                 {
                     "Value": "redhat-coreos-maipo-47.33-hvm",
                     "Key": "Name"
                 }
             ],
             "Hypervisor": "xen",
             "EnaSupport": true,
             "SriovNetSupport": "simple",
             "ImageId": "ami-0fd0205c3c81d0412",
             "State": "available",
             "BlockDeviceMappings": [
                 {
                     "DeviceName": "/dev/xvda",
                     "Ebs": {
                         "Encrypted": false,
                         "DeleteOnTermination": true,
                         "VolumeType": "gp2",
                         "VolumeSize": 8,
                         "SnapshotId": "snap-08318a8024617da0b"
                     }
                 },
                 {
                     "DeviceName": "/dev/xvdb",
                     "VirtualName": "ephemeral0"
                 }
             ],
             "Architecture": "x86_64",
             "ImageLocation": "531415883065/redhat-coreos-maipo-47.33-hvm",
             "RootDeviceType": "ebs",
             "OwnerId": "531415883065",
             "RootDeviceName": "/dev/xvda",
             "CreationDate": "2018-10-30T15:27:07.000Z",
             "Public": false,
             "ImageType": "machine",
             "Name": "redhat-coreos-maipo-47.33-hvm"
         }
     ]
  }

For the previous pipeline, see the output in d01ac5d (pkg/rhcos:
Default to the most-recent AMI, 2018-09-19, openshift#290).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants