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

Support Alibaba Cloud provider for add_cloud_metadata proccessor #4111

Merged

Conversation

athom
Copy link
Contributor

@athom athom commented Apr 25, 2017

Follow PR of #4023
Aims to support Alibaba Cloud provider for add_cloud_metadata processor.
Similar with Tencent Cloud, Alibaba's metadata services requires multiple requests too.
Here is the official document.
Tested on an esc instance with os ubuntu 16.04.
Note, Alibaba cloud's metadata services works only when the instance's network type is VPC.

@elasticmachine
Copy link
Collaborator

Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run.

1 similar comment
@elasticmachine
Copy link
Collaborator

Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run.

// Alibaba Cloud Metadata Service
// Document https://help.aliyun.com/knowledge_detail/49122.html
func newAlibabaCloudMetadataFetcher(c common.Config) (*metadataFetcher, error) {
ecsMetadataHost := "100.100.100.200"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's interesting that this is a fixed IP and not a domain name. Couldn't make much sense of the docs above unfortunately ;-)

Copy link
Contributor Author

@athom athom Apr 25, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, maybe metadata is not a really important feature for Chinese users, the design looks not so serious in both Tencent Cloud and Alibaba Cloud :-)

Couldn't make much sense of the docs above unfortunately ;-)

You mean remove the document comment?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment is fine.

@@ -359,6 +360,22 @@ _Tencent Cloud_
}
-------------------------------------------------------------------------------

_Alibaba Cloud_

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realize that this is supposed to just be examples of the output, but it would be good to mention something like, "Only when VPC is selected as the network type of the instance, you can get the metadata."

@dedemorton Maybe you have wording or formatting suggestions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's change the wording because we don't want to take the text verbatim from the Alibaba docs. How about adding this before the example:

This metadata is only available when VPC is selected as the network type of the instance.

Or, assuming we are talking about an ECS instance here (apologies for my ignorance), this would be even better and less confusing for users:

This metadata is only available when VPC is selected as the network type of the ECS instance.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added on the position above the example output.

@@ -3,6 +3,7 @@ package add_cloud_metadata
import "github.com/elastic/beats/libbeat/common"

// Tenccent Cloud Metadata Service
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/Tenccent/Tencent/?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 good catch!

Copy link
Member

@andrewkroh andrewkroh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@andrewkroh
Copy link
Member

jenkins, test it

@ruflin
Copy link
Member

ruflin commented Apr 27, 2017

jenkins, test it

@athom
Copy link
Contributor Author

athom commented Apr 27, 2017

Did my changes affect symlink rotate feature?
Looks an unrelated test case block here twice?
I don't have any idea to fix it for now, any thoughts?

@ruflin
Copy link
Member

ruflin commented Apr 27, 2017

@athom no, change is not related to the CI failure.

@andrewkroh andrewkroh merged commit 1344e4f into elastic:master Apr 27, 2017
@athom athom deleted the support-alibaba-cloud-for-add-cloud-metadata branch April 28, 2017 03:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants