Remove huaweicloud - revert #27607 #35184

mstinsky · 2023-04-24T08:59:55Z

Bug

What does this PR do?

This PR removes the huaweicloud from beats. The huaweicloud is just a openstack installation so since PR #27607 got merged all public and private openstack clouds can get detected as huaweicloud.
All the following lines from the huawei part are plain openstack [1] and are avaiable by every openstack metadata agent.

[1] https://github.com/elastic/beats/blob/main/libbeat/processors/add_cloud_metadata/provider_huawei_cloud.go#L49-L50

Why is it important?

As described above all openstack clouds may get detected as a huaweicloud.

Checklist

My code follows the style guidelines of this project
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have made corresponding change to the default configuration files
I have added tests that prove my fix is effective or that my feature works
I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Related issues

Closes Wrong detection of cloud.provider since Beats 8.0 (openstack as huawei) #31022

The huaweicloud is just openstack therefore revert #27607 to fix detection of all public and private openstack installations.

cla-checker-service · 2023-04-24T09:00:00Z

💚 CLA has been signed

mergify · 2023-04-24T09:00:30Z

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @mstinsky? 🙏.
For such, you'll need to label your PR with:

The upcoming major version of the Elastic Stack
The upcoming minor version of the Elastic Stack (if you're not pushing a breaking change)

To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

backport-v8./d.0 is the label to automatically backport to the 8./d branch. /d is the digit

elasticmachine · 2023-04-24T09:04:49Z

❕ Build Aborted

Either there was a build timeout or someone aborted the build.

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Duration: 4 min 29 sec

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.
/package : Generate the packages and run the E2E tests.
/beats-tester : Run the installation tests with beats-tester.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

belimawr · 2023-05-02T09:04:38Z

@elastic/obs-cloud-monitoring I believe this processor belongs to your team. Could someone take a look at it?

@mstinsky I see you haven't signed the Contributor Agreement yet. Could you follow the instructions on the link and sign it?

mstinsky · 2023-05-02T09:19:53Z

@belimawr thats strange I signed the license agreement last week when I opened this PR. Do I need to do anything that it registers here correctly?

kaiyan-sheng · 2023-05-02T22:01:20Z

Thanks @mstinsky for your contribution. Is there a way to fix the issue instead of removing huaweicloud support? This would be a breaking change for users who are using this add_cloud_metadata processor for huaweicloud.

mstinsky · 2023-05-02T22:09:53Z

@kaiyan-sheng I am an operator of an openstack cloud and not using any huaweicloud service. As such I have no idea of the internal working of the huaweicloud other then that the huaweicloud is using openstack.
If I look into the implementation of the huaweicloud processor it just plain identifies that huawei is running openstack. As long as huawei does not patch the openstack metadata agent to expose any kind of special identifier there is no way to differentiate between a 'normal' openstack installation and the openstack installation of huaweicloud (as its just openstack :D).

mstinsky · 2023-05-04T09:06:08Z

I was thinking about this issue a bit more and have another arguement to remove huaweicloud from beats @kaiyan-sheng. There are a lot of other public available clouds that use openstack under the hood like OVH, Open Telekom Cloud, Cleura, VEXXHOST and many more.
If we want to have 'special' support for the huaweicloud's openstack installation how do we manage every other public available openstack cloud that wants to have support in beats? My arguement is everything is openstack so it makes sense (at least to me) that all just gets detected as plain openstack.

mergify · 2023-06-27T03:17:54Z

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b remove-huawei-cloud upstream/remove-huawei-cloud
git merge upstream/main
git push upstream remove-huawei-cloud

andrewkroh · 2023-08-02T18:23:03Z

Like @mstinsky highlighted, the implementation is literally checking if it is running inside of openstack and then labeling it as being huawei. That is clearly insufficient to identity Huawei. It needs to have additional selection criteria (like checking another API that is huawei specific) OR huawei should be disabled by default OR removed entirely. (My least favorite fix is to disable it by default because the implementation would still be broken. We would have to disallow enabling openstack and huawei detection at the same time.)

beats/libbeat/processors/add_cloud_metadata/provider_huawei_cloud.go

Lines 49 to 50 in 90cd631

    
           metadataHost := "169.254.169.254" 
        
           huaweiCloudMetadataJSONURI := "/openstack/latest/meta_data.json"

mstinsky · 2023-09-08T12:03:16Z

@andrewkroh whats the plan here? Do we move forward with removing huaweicloud?

andrewkroh

It would be a breaking change to remove huawei as a configuration value for providers. Existing users that have huawei explicitly in their config should continue working. Perhaps we could internally treat huawei an alias for openstack. If both openstack and huawei are configured, then the implementation would need to ensure that only one provider actually gets instantiated .

In the docs I think it should probably state that

huawei is an alias for openstack. Hauwei Cloud operates on OpenStack. The cloud.provider will be reported as openstack.

In the release notes (see CHANGELOG.next.asciidoc) we would want to put an entry into the breaking changes section.

add_cloud_metadata - The huawei provider is now treated as openstack. Huawei Cloud runs on OpenStack, and from a metadata API perspective it is indistinguishable from OpenStack. If you know that your deployments only run on Huawei Cloud and you want the cloud.provider value to continue reporting as huawei, then you could overwrite the value using a processor (e.g. add_fields:{"when.equals.cloud.provider":"openstack","target":"cloud","fields":{"provider":"huawei"}}}). {issue}31022[31022] {pull}35184[35184]

Also fwiw, I am not the codeowner on this processor so the final approval will be in someone else's hands.

mstinsky · 2023-11-03T09:24:44Z

Can we get any feedback here from a codeowner on how to proceed with this issue?

andrewkroh · 2023-11-08T03:46:41Z

@elastic/obs-cloud-monitoring WDYT about the approach I laid out in my previous comment for removing the huawei provider?

kaiyan-sheng · 2023-11-08T20:14:53Z

@andrewkroh I think treating huawei as an alias and removing it should work at least for the short term. We are trying to prioritize the work of fixing the add_cloud_metadata processor but no clue if we will even find a solution for huawei vs OpenStack tbh.

kaiyan-sheng · 2023-11-09T22:38:10Z

@andrewkroh WDYT about instead of completely removing huaweiMetadataFetcher, we keep it and only use this fetcher when huawei is specified using the providers config parameter?

Something like this:

diff --git a/libbeat/processors/add_cloud_metadata/providers.go b/libbeat/processors/add_cloud_metadata/providers.go
index 2b9f0d9064..022784399b 100644
--- a/libbeat/processors/add_cloud_metadata/providers.go
+++ b/libbeat/processors/add_cloud_metadata/providers.go
@@ -64,7 +64,7 @@ var cloudMetaProviders = map[string]provider{
        "nova-ssl":      openstackNovaSSLMetadataFetcher,
        "qcloud":        qcloudMetadataFetcher,
        "tencent":       qcloudMetadataFetcher,
-       "huawei":        huaweiMetadataFetcher,
+       "huawei":        openstackNovaMetadataFetcher,
        "hetzner":       hetznerMetadataFetcher,
 }
 
@@ -91,7 +91,11 @@ func filterMetaProviders(filter func(string) bool, fetchers map[string]provider)
        out := map[string]provider{}
        for name, ff := range fetchers {
                if filter(name) {
-                       out[name] = ff
+                       if name == "huawei" {
+                               out[name] = huaweiMetadataFetcher
+                       } else {
+                               out[name] = ff
+                       }
                }
        }
        return out

andrewkroh · 2023-11-10T20:18:45Z

WDYT about instead of completely removing huaweiMetadataFetcher, we keep it and only use this fetcher when huawei is specified using the providers

That could work (not commenting on the patch), the processor would need to enforce a restriction that huawei cannot be used along side any of the openstack providers (openstackNovaMetadataFetcher, openstackNovaSSLMetadataFetcher).

mstinsky · 2023-11-11T15:28:56Z

I am neither a go developer nor do I have good knowledge about the filebeat codebase.
But as the huaweiMetadataFetcher is essentially doing the same thing as the openstackNovaMetadataFetcher, I would argue that keeping the code is redundant and opens the door for more bugs down the line when handling it in a special way.

Also I would like to bring up my argument from #35184 (comment) once more.
There are many more public openstack clouds out there, having a special case for huawei opens the door to support numerous other openstack clouds. And I am not sure if beats wants to do that.

kaiyan-sheng · 2023-11-13T19:51:20Z

@mstinsky @andrewkroh Thank you both for the comments! I agree with your argument about having special case for huawei can be problematic in the future. Let's make huawei an alias for OpenStack and keep it as a provider option, so we are not breaking anyone then!

mstinsky · 2023-11-15T11:19:35Z

@kaiyan-sheng would you be willing to provide a patch for that?
I am not sure what you both mean with keeping huawei as an alias. As I said I dont have much experience with go, if you want me to provide a patch for that I would need a bit of guidance :)

elasticmachine · 2024-01-29T22:14:32Z

❕ Build Aborted

The PR is not allowed to run in the CI yet

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2024-01-29T22:09:37.794+0000
Duration: 5 min 50 sec

Steps errors

Expand to view the steps failures

`Load a resource file from a library`

Took 0 min 0 sec . View more details here
Description: approval-list/elastic/beats.yml

`Error signal`

Took 0 min 0 sec . View more details here
Description: githubApiCall: The REST API call https://api.github.com/orgs/elastic/members/mstinsky return the message : java.lang.Exception: httpRequest: Failure connecting to the service https://api.github.com/orgs/elastic/members/mstinsky : httpRequest: Failure connecting to the service https://api.github.com/orgs/elastic/members/mstinsky : Code: 404Error: {"message":"User does not exist or is not a member of the organization","documentation_url":"https://docs.github.com/rest/orgs/members#check-organization-membership-for-a-user"}

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.
/package : Generate the packages and run the E2E tests.
/beats-tester : Run the installation tests with beats-tester.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

elasticmachine · 2024-01-30T15:44:17Z

❕ Build Aborted

Either there was a build timeout or someone aborted the build.

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Duration: 5 min 54 sec

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.
/package : Generate the packages and run the E2E tests.
/beats-tester : Run the installation tests with beats-tester.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

elasticmachine · 2024-01-31T23:22:50Z

❕ Build Aborted

Either there was a build timeout or someone aborted the build.

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Duration: 66 min 27 sec

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.
/package : Generate the packages and run the E2E tests.
/beats-tester : Run the installation tests with beats-tester.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

elasticmachine · 2024-02-01T01:26:48Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2024-01-31T23:15:28.951+0000
Duration: 131 min 4 sec

Test stats 🧪

Test	Results
Failed	0
Passed	28776
Skipped	2014
Total	30790

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.
/package : Generate the packages and run the E2E tests.
/beats-tester : Run the installation tests with beats-tester.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

elasticmachine · 2024-02-01T04:56:52Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2024-02-01T02:42:02.331+0000
Duration: 135 min 34 sec

Test stats 🧪

Test	Results
Failed	0
Passed	28780
Skipped	2014
Total	30794

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.
/package : Generate the packages and run the E2E tests.
/beats-tester : Run the installation tests with beats-tester.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

elasticmachine · 2024-02-01T16:18:32Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2024-02-01T14:06:18.205+0000
Duration: 132 min 52 sec

Test stats 🧪

Test	Results
Failed	0
Passed	28780
Skipped	2014
Total	30794

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.
/package : Generate the packages and run the E2E tests.
/beats-tester : Run the installation tests with beats-tester.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

* Remove huaweicloud - revert #27607 The huaweicloud is just openstack therefore revert #27607 to fix detection of all public and private openstack installations. * make huawei an alias for openstack * change doc * Update CHANGELOG.next.asciidoc --------- Co-authored-by: kaiyan-sheng <[email protected]> (cherry picked from commit 07c559b) # Conflicts: # libbeat/processors/add_cloud_metadata/docs/add_cloud_metadata.asciidoc # libbeat/processors/add_cloud_metadata/provider_huawei_cloud.go # libbeat/processors/add_cloud_metadata/provider_huawei_cloud_test.go # libbeat/processors/add_cloud_metadata/providers.go

* Remove huaweicloud - revert #27607 The huaweicloud is just openstack therefore revert #27607 to fix detection of all public and private openstack installations. * make huawei an alias for openstack * change doc * Update CHANGELOG.next.asciidoc --------- Co-authored-by: kaiyan-sheng <[email protected]> (cherry picked from commit 07c559b)

* Remove huaweicloud - revert #27607 (#35184) * Remove huaweicloud - revert #27607 The huaweicloud is just openstack therefore revert #27607 to fix detection of all public and private openstack installations. * make huawei an alias for openstack * change doc * Update CHANGELOG.next.asciidoc --------- Co-authored-by: kaiyan-sheng <[email protected]> (cherry picked from commit 07c559b) * Update CHANGELOG.next.asciidoc * fix footnote --------- Co-authored-by: Maximilian Stinsky <[email protected]> Co-authored-by: kaiyan-sheng <[email protected]>

* Remove huaweicloud - revert #27607 (#35184) * Remove huaweicloud - revert #27607 The huaweicloud is just openstack therefore revert #27607 to fix detection of all public and private openstack installations. * make huawei an alias for openstack * change doc * Update CHANGELOG.next.asciidoc --------- Co-authored-by: kaiyan-sheng <[email protected]> (cherry picked from commit 07c559b) # Conflicts: # libbeat/processors/add_cloud_metadata/docs/add_cloud_metadata.asciidoc # libbeat/processors/add_cloud_metadata/provider_huawei_cloud.go # libbeat/processors/add_cloud_metadata/provider_huawei_cloud_test.go # libbeat/processors/add_cloud_metadata/providers.go * Update CHANGELOG.next.asciidoc * Update add_cloud_metadata.asciidoc * Update providers.go * fix doc * fix lint --------- Co-authored-by: Maximilian Stinsky <[email protected]> Co-authored-by: kaiyan-sheng <[email protected]>

* Remove huaweicloud - revert elastic#27607 The huaweicloud is just openstack therefore revert elastic#27607 to fix detection of all public and private openstack installations. * make huawei an alias for openstack * change doc * Update CHANGELOG.next.asciidoc --------- Co-authored-by: kaiyan-sheng <[email protected]>

Remove huaweicloud - revert #27607

82d90d8

The huaweicloud is just openstack therefore revert #27607 to fix detection of all public and private openstack installations.

mstinsky requested a review from a team as a code owner April 24, 2023 08:59

mstinsky requested review from belimawr and fearful-symmetry and removed request for a team April 24, 2023 08:59

botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Apr 24, 2023

mergify bot assigned mstinsky Apr 24, 2023

andrewkroh mentioned this pull request Aug 2, 2023

Wrong detection of cloud.provider since Beats 8.0 (openstack as huawei) #31022

Closed

andrewkroh reviewed Sep 8, 2023

View reviewed changes

make huawei an alias for openstack

73d1a2e

kaiyan-sheng requested a review from a team as a code owner November 15, 2023 22:06

Merge branch 'main' into remove-huawei-cloud

c7bd503

kaiyan-sheng self-assigned this Jan 29, 2024

Merge branch 'main' into remove-huawei-cloud

6ef34ee

kaiyan-sheng requested a review from andrewkroh January 30, 2024 15:38

kaiyan-sheng added the Team:Cloud-Monitoring Label for the Cloud Monitoring team label Jan 30, 2024

botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Jan 30, 2024

andrewkroh approved these changes Jan 31, 2024

View reviewed changes

kaiyan-sheng added 2 commits January 31, 2024 15:15

Merge branch 'main' into remove-huawei-cloud

9ae4725

Merge branch 'main' into remove-huawei-cloud

2d113a5

kaiyan-sheng enabled auto-merge (squash) February 1, 2024 02:41

Merge branch 'main' into remove-huawei-cloud

dd9909a

kaiyan-sheng added backport-v8.12.0 Automated backport with mergify backport-7.17 Automated backport to the 7.17 branch with mergify labels Feb 1, 2024

Merge branch 'main' into remove-huawei-cloud

84c157e

kaiyan-sheng approved these changes Feb 1, 2024

View reviewed changes

kaiyan-sheng merged commit 07c559b into elastic:main Feb 1, 2024
86 checks passed

This was referenced Feb 1, 2024

[7.17](backport #35184) Remove huaweicloud - revert #27607 #37828

Merged

[8.12](backport #35184) Remove huaweicloud - revert #27607 #37829

Merged

kilfoyle mentioned this pull request Feb 1, 2024

Remove footnote to fix broken docs build #37832

Merged

Remove huaweicloud - revert #27607 #35184

Remove huaweicloud - revert #27607 #35184

Conversation

mstinsky commented Apr 24, 2023

What does this PR do?

Why is it important?

Checklist

Related issues

cla-checker-service bot commented Apr 24, 2023 • edited Loading

mergify bot commented Apr 24, 2023

elasticmachine commented Apr 24, 2023 • edited by jenkins-beats-ci bot Loading

❕ Build Aborted

Build stats

🤖 GitHub comments

belimawr commented May 2, 2023

mstinsky commented May 2, 2023

kaiyan-sheng commented May 2, 2023

mstinsky commented May 2, 2023

mstinsky commented May 4, 2023

mergify bot commented Jun 27, 2023

andrewkroh commented Aug 2, 2023 • edited Loading

mstinsky commented Sep 8, 2023

andrewkroh left a comment • edited Loading

Choose a reason for hiding this comment

mstinsky commented Nov 3, 2023

andrewkroh commented Nov 8, 2023

kaiyan-sheng commented Nov 8, 2023

kaiyan-sheng commented Nov 9, 2023

andrewkroh commented Nov 10, 2023

mstinsky commented Nov 11, 2023

kaiyan-sheng commented Nov 13, 2023

mstinsky commented Nov 15, 2023

elasticmachine commented Jan 29, 2024

❕ Build Aborted

Build stats

Steps errors

Load a resource file from a library

Error signal

🤖 GitHub comments

elasticmachine commented Jan 30, 2024

❕ Build Aborted

Build stats

🤖 GitHub comments

elasticmachine commented Jan 31, 2024

❕ Build Aborted

Build stats

🤖 GitHub comments

elasticmachine commented Feb 1, 2024

💚 Build Succeeded

Build stats

Test stats 🧪

💚 Flaky test report

🤖 GitHub comments

elasticmachine commented Feb 1, 2024

💚 Build Succeeded

Build stats

Test stats 🧪

💚 Flaky test report

🤖 GitHub comments

elasticmachine commented Feb 1, 2024

💚 Build Succeeded

Build stats

Test stats 🧪

💚 Flaky test report

🤖 GitHub comments

cla-checker-service bot commented Apr 24, 2023 •

edited

Loading

elasticmachine commented Apr 24, 2023 •

edited by jenkins-beats-ci bot

Loading

andrewkroh commented Aug 2, 2023 •

edited

Loading

andrewkroh left a comment •

edited

Loading

`Load a resource file from a library`

`Error signal`