-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Add Openstack Nova cloud_metadata Processor (#7649) #7663
Conversation
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
// specific language governing permissions and limitations | ||
// under the License. | ||
|
||
package add_cloud_metadata |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't use an underscore in package name
// specific language governing permissions and limitations | ||
// under the License. | ||
|
||
package add_cloud_metadata |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't use an underscore in package name
// Openstack Nova Metadata Service | ||
// Document https://docs.openstack.org/nova/latest/user/metadata-service.html | ||
func newOpenstackNovaMetadataFetcher(c *common.Config) (*metadataFetcher, error) { | ||
osMetadataHost := "169.254.169.254" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a constant for this named metadataHost
.
// Document https://docs.openstack.org/nova/latest/user/metadata-service.html | ||
func newOpenstackNovaMetadataFetcher(c *common.Config) (*metadataFetcher, error) { | ||
osMetadataHost := "169.254.169.254" | ||
osMetadataInstanceIDURI := "/2009-04-04/meta-data/instance-id" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might as well make this constants.
if err != nil { | ||
return nil, err | ||
} | ||
fmt.Println(urls) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fmt.Println
needs removed.
return nil | ||
}, | ||
urls[2]: func(all []byte, result *result) error { | ||
result.metadata["hostname"] = string(all) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hostname isn't in the schema for this processor. Is it different than the machine's hostname? Do we need it?
"github.com/elastic/beats/libbeat/common" | ||
) | ||
|
||
// Openstack Nova Metadata Service |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
godoc method docs should start with the method name. "newOpenstackNovaMetadataFetcher returns a metadataFetcher for the OpenStack Nova Metadata Service. ..."
So if both metadata services (AWS and OpenStack) are available what happens? IIRC the first one to return a response wins. Perhaps it now needs a way to configure what providers are enabled. |
Indeed... I can use the native Openstack data if preferred but this endpoint isn't on par with the EC2-compatible one (it doesn't provide the machine-type for instance) |
I assume the above would happen if OpenStack is run on AWS? Would we expect both to return the same data? Could you also add a changelog entry? |
I don't really the point of running Openstack on AWS, but if that was the case, I assume that the network component of Openstack would provide some overlay network and thus the metadata endpoint would be the Openstack one. I think that what @andrewkroh thinks about is if we run on AWS, then both metadata providers will be "active" (as both endpoint will return valid responses). For the changelog entry, shall I add them to |
For the changelog, please add it to Thanks for the explanation of AWS / OpenStack mix. I assume that the data format was the same but that the url to the API endpoints would be different so only 1 would work. But that's not the case? |
Indeed, both providers use the exact same endpoints 😿 So this morning I spun an instance on EC2 just to see which metadata is added and it appears that only the first matching provider is used (I didn't dig in the code to find where)
|
|
||
func initOpenstackNovaTestServer() *httptest.Server { | ||
return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
if r.RequestURI == "/2009-04-04/meta-data/instance-id" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could use here and below the defined constants from above I think
I thought of one more thingthat needs updated. In |
@andrewkroh thanks for the pointer, I've added the example to the doc as well ! |
jenkins test this please |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks!
I'm considering opening an issue and accompanying merge request. I believe the AWS compatible instance id should have been stored under a key identifying it as such. OpenStack users do not work with the EC2 compatible instance ID and will find it foreign. It doesn't match the ID of the API, CLI, or Web Console. I'm proposing moving the EC2 compatible instance id to a key named ec2_instance_id, and exposing the OpenStack instance_id under instance_id. However, I realize this would be a potentially breaking change for existing users. Any suggestions for handling the naming conflict? I could name the OpenStack instance id, 'id'. While I'm at it, I'll add some other fields such as project id. |
Using the EC2 compatible metadata endpoint because it exposes more useful information that the native "openstack" endpoint.