search

cloudfoundry-attic / bosh-notes

Collection of proposals for BOSH
Apache License 2.0
51 stars 23 forks source link

Include metadata as part of create_vm and create_disk #48

Open nehagjain15 opened 5 years ago

cfdreddbot commented 5 years ago

Hey nehagjain15!

Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you and the commit authors have already signed the CLA.

voelzmo commented 5 years ago

I'm not sure I fully understand the motivation behind this. So far, we have ensured that after creating a VM and after creating a disk the Director calls the method to set the metadata. This sets all IaaS specific tags on the infrastructures which support it.

yeshwantbabar commented 5 years ago

Hi @voelzmo -

Sorry about the delay on getting back to you.

As Neha mentioned there are similar requests for the other IaaSes as well.

What we are looking for is the ability to accept a prefix/suffix strings that can be specified in the VMTypes / Deployment manifest so that all the VMs that belong to a particular Instance Group get a certain prefix or suffix added to their names. There are several usecases for these -

  1. Providing some context for what the VM does for admins.
  2. Abiding by the existing standards for what the VM names should look like.
  3. Being able create folder structure based on deployments with the folder hierarchy having the name that's human readable.
  4. These names can be plugged into the automation frameworks like powercli.

Please let me know your thoughts.

cc: @mfine30

dpb587-pivotal commented 5 years ago

@friegger @beyhan @voelzmo - forgot to mention this earlier today. Do you have any objections around this potential CPI change to provide VM metadata as part of the initial create_vm call? This came up again in some recent discussions.

A related issue was raised in the AWS CPI as an example of how it could be used in other IaaSes. https://github.com/cloudfoundry/bosh-aws-cpi-release/issues/93 refers to locking down EC2 operations based on tags (and those tags would need to be present on the initial create_vm/RunInstances call).

@voelzmo mentions that cloudfoundry/bosh#1163 is orthogonal, which I'd mostly agree. I think there's the caveat that some IaaSes might not support renaming things after the resource is created, and this would help that situation. I think this is the case in vCenter for VM names where they can't be changed after creation. (Whether or not that property should be a director-level property is a different topic)

Anyways, I'd rather consider implementing this change before CPIs start relying on the groups field of the API which is not an officially ordered list of metadata. That field could technically change in the future, and I don't want director to be tightly tied to keeping the exact values.

nehagjain15 commented 5 years ago

@voelzmo if we get metadata as part of create_vm call we could

  1. Name VM's appropriately instead of using uuid
  2. We could also create folder based structure and group VM's based on deployment_name/ instance_group name.
beyhan commented 5 years ago

I don't have any. I would like to have VM creation and tagging possible with one call on BOSH side. CPIs could decide whether they want/can support this or not.

voelzmo commented 5 years ago

@dpb587-pivotal:

Do you have any objections around this potential CPI change to provide VM metadata as part of the initial create_vm call?

Sounds good to me. OpenStack does allow for setting metadata on create_vm and create_volume, so this should also reduce calls on the OpenStack CPI side.

With the questions above I just wanted to understand where the request is coming from: Which problem are we solving and for whom? I'm perfectly fine with helping vSphere pick better names for VMs and folders and also help lock down security on AWS. This would most likely also help with better VM names for GCP: https://github.com/cloudfoundry/bosh-google-cpi-release/issues/217#issuecomment-336978801. Thanks for following up on this and helping me understand!

nehagjain15 commented 5 years ago

@voelzmo & @dpb587-pivotal I updated the proposal based on your conversation. What would be the next steps to get this added to tracker so that the change can be implemented?

dpb587-pivotal commented 5 years ago

Reposting from internal Slack where I didn't hear any response...

From my perspective, I think there's enough of an understanding of what changes are necessary and desired for it, and I think it makes sense. Conceptually, I think the two pieces are:

1) implement on director for create_vm, create_disk, create_snapshots; the API looks like it'll be backwards compatible for at least ruby+go CPIs (but we'll want to verify that during implementation). 2) individual CPIs could then implement it. Depending on who is implementing the director changes, the AWS/vSphere would probably be implemented alongside it for testing.

@nehagjain15 @yeshwantbabar, I know you're interested in the vSphere CPI support, at least. Are you also interested in implementing the director-side stuff as well? Otherwise, I'd recommend you and @mfine30 chat more about where it'd fall in prioritization between your teams, here or in your next sync.

voelzmo commented 5 years ago

@dpb587-pivotal I'd still be interested in a concept on how to avoid unnecessary calls from director to CPI (and therefore to the IaaS API). This would also help with other issues e.g. rate limiting on AWS.

dpb587-pivotal commented 5 years ago

@voelzmo seems reasonable. Want to document your thoughts here, or discuss it elsewhere? And do you want it to be a blocker on any initial release of CPI+director for this functionality, or would you consider it something that can be improved on later?