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

Property names are different with the google CPI than others #31

Closed
dsboulder opened this issue Jul 11, 2016 · 14 comments
Closed

Property names are different with the google CPI than others #31

dsboulder opened this issue Jul 11, 2016 · 14 comments
Assignees

Comments

@dsboulder
Copy link
Member

Hey GCP CPI developers -

I noticed some inconsistencies when using the google CPI next to the AWS, OpenStack, and vSphere CPIs. There's a few properties that aren't named the same with the google CPI, here's what they are:

  1. The bosh-init cloud_provider agent: properties section seems to expect blobstore and ntp settings. Usually these settings are at the root level and are pulled in by the other CPIs.
  2. The bosh-init job properties agent section has a similar inconsistency in it. Usually the blobstore and ntp settings are not inside the agent section.
  3. The local bosh-init blobstore path in your CPI is provided in an additional namespace called blobstore_options like this: {provider: "local", options: {blobstore_path: "/var/vcap/micro_bosh/data/cache"}}. Other CPIs have the blobstore path more at the root level like this: {provider: "local", path: "/var/vcap/micro_bosh/data/cache"}

If y'all help make those changes, it'll be much easier for existing BOSH users of another IaaS to learn GCP. I've already had 5 or 6 people run into this, including me. Thanks!!!

@evandbrown evandbrown self-assigned this Jul 13, 2016
@evandbrown
Copy link
Contributor

Howdy @dsboulder,

It looks like the bosh-init manifest parsing is pretty loose when handling cloud_provider.properties: it unmarshals properties to a map. I'm guessing that is plumbed into the CPI here, but I'm not seeing anything else in the CPI where we use settings["blobstore_path"] or similar. Are these values passed directly to the bosh-agent, and it's being flexible with the values it receives?

@dsboulder
Copy link
Member Author

@evandbrown I think you're right about blobstore_path, that must have been a change to the director/agent that was totally unrelated to this particular CPI. You okay making the other changes though?

evandbrown added a commit that referenced this issue Jul 14, 2016
Addresses #31 by restructuring the mbus, ntp, and blobstore configuration
properties to be consistent with existing CPIs.
evandbrown added a commit that referenced this issue Jul 14, 2016
Addresses #31 by restructuring the mbus, ntp, and blobstore configuration
properties to be consistent with existing CPIs.
evandbrown added a commit that referenced this issue Jul 14, 2016
Addresses #31 by restructuring the mbus, ntp, and blobstore configuration
properties to be consistent with existing CPIs.
evandbrown added a commit that referenced this issue Jul 14, 2016
Addresses #31 by restructuring the mbus, ntp, and blobstore configuration
properties to be consistent with existing CPIs.
evandbrown added a commit that referenced this issue Jul 14, 2016
Addresses #31 by restructuring the mbus, ntp, and blobstore configuration
properties to be consistent with existing CPIs.
evandbrown added a commit that referenced this issue Jul 14, 2016
Addresses #31 by restructuring the mbus, ntp, and blobstore configuration
properties to be consistent with existing CPIs.
evandbrown added a commit that referenced this issue Jul 14, 2016
Addresses #31 by restructuring the mbus, ntp, and blobstore configuration
properties to be consistent with existing CPIs.
@evandbrown
Copy link
Contributor

@dsboulder Clarifying question: the bosh-agent uses the blobstore and ntp config (https://github.com/cloudfoundry/bosh-agent/blob/master/settings/settings.go), so it feels right that they're nested in the agent property. But then I see the job spec for the AWS CPI, and it has both a agent.blobstore as well as a top level blobstore. Is there a difference? Should we have both in the Google CPI?

It was easy to reorder the struct fields, and I think all that's left is the job spec's cpi.json.erb, I just want to be sure there wasn't a good reason for nesting this under agent in the first place. Thoughts?

evandbrown added a commit that referenced this issue Jul 14, 2016
Addresses #31 by restructuring the mbus, ntp, and blobstore configuration
properties to be consistent with existing CPIs.
@dsboulder
Copy link
Member Author

@Cpp4Life what do you think?

evandbrown added a commit that referenced this issue Jul 15, 2016
Addresses #31 by restructuring the mbus, ntp, and blobstore configuration
properties to be consistent with existing CPIs.
@dsboulder
Copy link
Member Author

dsboulder commented Jul 15, 2016

@cppforlife for real. Dmitriy might have a plan to normalize them better, I agree with you Eric that it makes sense under Agent, it's just not what the other ones do.
Also @ljfranklin is the anchor of the CPI team

@ljfranklin
Copy link
Contributor

@dsboulder @evandbrown The vSphere and AWS CPIs allow the user to specify blobstore + ntp either under agent or at the root. That said, when we actually generate the cpi.json file, we always nest these properties under agent. Agreed that it makes more sense to nest under agent, but all our bosh.io example bosh-init manifests show these props at the top level. Oh well, seems like allowing it in both places makes the most sense in practice.

@dsboulder
Copy link
Member Author

Maybe then the best solution is to allow parameters to come in with either name. What do you think @evandbrown?

@evandbrown
Copy link
Contributor

@dsboulder totally agree. @ljfranklin's comment and a convo w/ Corrie today made things crystal clear. should have this working very shortly...

@evandbrown
Copy link
Contributor

@dsboulder #37 (pending BATS and int tests) supports a template with cloud_provider.properties like this

  properties:
    google: *google_properties
    agent: {mbus: "https://mbus:mbus-password@0.0.0.0:6868"}
    blobstore: {provider: local, path: /var/vcap/micro_bosh/data/cache}
    ntp: *ntp

Does this look correct?

Full template here:

---
name: bosh

releases:
  - name: bosh
    url: https://bosh.io/d/github.com/cloudfoundry/bosh?v=256.2
    sha1: ff2f4e16e02f66b31c595196052a809100cfd5a8
  - name: bosh-google-cpi
    url: file:///home/evanbrown/dev/cloudfoundry/bosh-google-cpi-release/dev_releases/bosh-google-cpi/bosh-google-cpi-21+dev.10.tgz

resource_pools:
  - name: vms
    network: private
    stemcell:
      url: file://stemcell.tgz
    cloud_properties:
      machine_type: n1-standard-4
      root_disk_size_gb: 40
      root_disk_type: pd-standard
      service_scopes:
        - compute
        - devstorage.full_control

disk_pools:
  - name: disks
    disk_size: 32_768
    cloud_properties:
      type: pd-standard

networks:
  - name: vip
    type: vip
  - name: private
    type: manual
    subnets:
    - range: 10.0.0.0/29
      gateway: 10.0.0.1
      static: [10.0.0.3-10.0.0.7]
      cloud_properties:
        network_name: cf
        subnetwork_name: bosh-us-east1
        ephemeral_external_ip: true
        tags:
          - bosh-internal

jobs:
  - name: bosh
    instances: 1

    templates:
      - name: nats
        release: bosh
      - name: postgres
        release: bosh
      - name: powerdns
        release: bosh
      - name: blobstore
        release: bosh
      - name: director
        release: bosh
      - name: health_monitor
        release: bosh
      - name: google_cpi
        release: bosh-google-cpi

    resource_pool: vms
    persistent_disk_pool: disks

    networks:
      - name: private
        static_ips: [10.0.0.6]
        default:
          - dns
          - gateway

    properties:
      nats:
        address: 127.0.0.1
        user: nats
        password: nats-password

      postgres: &db
        listen_address: 127.0.0.1
        host: 127.0.0.1
        user: postgres
        password: postgres-password
        database: bosh
        adapter: postgres

      dns:
        address: 10.0.0.6
        domain_name: microbosh
        db: *db
        recursor: 169.254.169.254

      blobstore:
        address: 10.0.0.6
        port: 25250
        provider: dav
        director:
          user: director
          password: director-password
        agent:
          user: agent
          password: agent-password

      director:
        address: 127.0.0.1
        name: micro-google
        db: *db
        cpi_job: google_cpi
        user_management:
          provider: local
          local:
            users:
              - name: admin
                password: admin
              - name: hm
                password: hm-password
      hm:
        director_account:
          user: hm
          password: hm-password
        resurrector_enabled: true

      google: &google_properties
        project: evandbrown17
        default_zone: us-east1

      agent:
        mbus: nats://nats:nats-password@10.0.0.6:4222

      ntp: &ntp
        - 169.254.169.254

cloud_provider:
  template:
    name: google_cpi
    release: bosh-google-cpi

  ssh_tunnel:
    host: 10.0.0.6
    port: 22
    user: bosh
    private_key: /home/evanbrown/.ssh/google_compute_engine

  mbus: https://mbus:mbus-password@10.0.0.6:6868

  properties:
    google: *google_properties
    agent: {mbus: "https://mbus:mbus-password@0.0.0.0:6868"}
    blobstore: {provider: local, path: /var/vcap/micro_bosh/data/cache}
    ntp: *ntp

evandbrown added a commit that referenced this issue Jul 19, 2016
all: Normalize cloud_provider options for bosh-init

This commit restructures the mbus, ntp, and blobstore configuration properties to be consistent with existing CPIs. Addresses #31.

A bosh-init manifest's cloud_provider.properties section should include the following keys:

google
agent
blobstore
`ntp
For example:

properties:
      google: *google_properties
      agent: {mbus: "https://mbus:mbus-password@0.0.0.0:6868"}
      blobstore: {provider: local, path: /var/vcap/micro_bosh/data/cache}
      ntp: *ntp
This is a departure from previous versions of the CPI that expectd agent and ntp keys to children of the agent key.
@dsboulder
Copy link
Member Author

I think that's correct! We'll know for sure when we put it in OpsManager next week or the week after. That'll expose any inconsistencies. If you'd like, we could build a CPI ourselves from this branch and use it first to verify that it fixes the naming issues.

@evandbrown
Copy link
Contributor

Cool! The branch is merged and released. CPI 24.0.0 (e2f77a0a8696b29fdb676cf447cfd9bc6841b648) with stemcell 3262.2 (f46d82a6ae6e89a5635cb3122389f0c8459a82e0) is the latest. Also, the README.md is now updated from CI with the current releases.

We'll keep this open until you ack everything's working!

@cunnie
Copy link
Member

cunnie commented Jul 23, 2016

Thanks @evandbrown for fixing — I got burned on this one, too. FYI, after reading this thread, I was able to deploy with the following changes to my [previously working] manifest — it doesn't quite match my AWS BOSH manifest, but it's close enough for me It matches my AWS manifest (which is a good thing):

     agent:
       mbus: https://mbus:mbus-password@0.0.0.0:6868
-      blobstore:
-        provider: local
-        options:
-          blobstore_path: /var/vcap/micro_bosh/data/cache
+    blobstore:
+      provider: local
+      path: /var/vcap/micro_bosh/data/cache

On a completely unrelated note, is specifying the subnetwork_name a requirement or am I no longer able to use relative paths to specify my .ssh key? I was unable to deploy successfully until I made the following additional modifications to my bosh-init manifest:

+      subnetwork_name: cf-e6ecf3fd8a498fbe
       tags:
       - cf-internal
       - cf-bosh
@@ -170,7 +171,7 @@ cloud_provider:
     host: 104.154.39.128 # <--- replace with External IP
     port: 22
     user: vcap # <--- replace with the user corresponding to your private ssh key
-    private_key: ../../.ssh/google # <--- replace with the location of your google_compute_engine ssh private key
+    private_key: /Users/cunnie/.ssh/google # <--- replace with the location of your google_compute_engine ssh private key

The error I received before making the above changes was this:

    Command 'deploy' failed:
      Deploying:
        Creating instance 'bosh/0':
          Waiting until instance is ready:
            Starting SSH tunnel:
              Failed to connect to remote server:
                ssh: handshake failed: read tcp 10.0.9.140:63996->104.154.39.128:22: read: connection reset by peer

@evandbrown
Copy link
Contributor

Thanks for the feedback, @cunnie! If there's anything we can do to make it more precisely match the AWS manifest, let me know and I'm happy to keep improving.

subnetwork_name is required. I recall that relative paths do not work; I'll open a separate issue to track that.

@cunnie
Copy link
Member

cunnie commented Jul 23, 2016

@evandbrown I was wrong: my Google BOSH manifest's cloud_provider.properties section matches exactly my AWS BOSH manifest's (with the exception of the aws and google sections) — you don't need to change anything. Sorry for the mix-up. I don't know where I originally pulled that section from (probably some old manifest somewhere).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants