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

Ensure kind cluster has RFC1123 compliant name #20627

Merged
merged 1 commit into from
Sep 1, 2020

Conversation

jarpy
Copy link
Contributor

@jarpy jarpy commented Aug 17, 2020

What does this PR do?

This patch:

  • Modifies the cluster name generation function to produce - where it previously produced _
  • Adds run-time validation that the generated name is compliant
  • Renames the function from kubernetesPodName to kubernetesClusterName, reflecting its purpose

Why is it important?

When creating a kind cluster, we must use a cluster name that is a valid Kubernetes resource name, and by extension, a valid DNS name. If not, kind cluster provisioning can fail when kind tries to create resources with invalid names.

For example, if trying to create a cluster called "a_b" (underscores are not permitted), control-plane provisioning will fail with:

host 'a_b-control-plane' must be [...] a valid RFC-1123 DNS subdomain

With this patch, the kubernetes Metricbeat module integration tests are able to run to completion on my system. Without it, kind fails in this manner:

$ kind create cluster --name metricbeat-8_0_0-a34234bc-master
Creating cluster "metricbeat-8_0_0-a34234bc-master" ...
 ✓ Ensuring node image (kindest/node:v1.18.2) 🖼 
 ✓ Preparing nodes 📦  
 ✓ Writing configuration 📜 
 ✗ Starting control-plane 🕹️ 
ERROR: failed to create cluster: failed to init node with kubeadm: command "docker exec --privileged metricbeat-8_0_0-a34234bc-master-control-plane kubeadm init --ignore-preflight-errors=all --config=/kind/kubeadm.conf --skip-token-print --v=6" failed with error: exit status 1
Command Output: I0817 07:52:20.014341     163 initconfiguration.go:200] loading configuration from "/kind/kubeadm.conf"
[config] WARNING: Ignored YAML document with GroupVersionKind kubeadm.k8s.io/v1beta2, Kind=JoinConfiguration
I0817 07:52:20.018540     163 interface.go:400] Looking for default routes with IPv4 addresses
I0817 07:52:20.018553     163 interface.go:405] Default route transits interface "eth0"
I0817 07:52:20.018623     163 interface.go:208] Interface eth0 is up
I0817 07:52:20.018673     163 interface.go:256] Interface "eth0" has 3 addresses :[172.20.0.2/16 fc00:f853:ccd:e793::2/64 fe80::42:acff:fe14:2/64].
I0817 07:52:20.018689     163 interface.go:223] Checking addr  172.20.0.2/16.
I0817 07:52:20.018696     163 interface.go:230] IP found 172.20.0.2
I0817 07:52:20.018702     163 interface.go:262] Found valid IPv4 address 172.20.0.2 for interface "eth0".
I0817 07:52:20.018706     163 interface.go:411] Found active IP 172.20.0.2 
hostport metricbeat-8_0_0-a34234bc-master-control-plane:6443: host 'metricbeat-8_0_0-a34234bc-master-control-plane' must be a valid IP address or a valid RFC-1123 DNS subdomain

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.

How to test this PR locally

Run the integ tests for the kubernetes Metricbeat module with Mage in verbose mode. kind will be seen to run correctly. Without the patch kind will fail to provision the control-plane.

MODULE=kubernetes mage -v integtest

With Mage not in verbose mode, the kubernetes integ test is not run, and no output is seen (because it dies at setup time).

When creating a kind cluster, we must use a cluster name that is a valid
Kubernetes resource name, and by extension, a valid DNS name. If not,
kind cluster provisioning can fail when kind tries to create resources
with invalid names.

For example, if trying to create a cluster called "a_b" (underscores
are not permitted), control-plane provisioning will fail with:

  host 'a_b-control-plane' must be [...] a valid RFC-1123 DNS subdomain
@jarpy jarpy added the bug label Aug 17, 2020
@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Aug 17, 2020
@andresrc andresrc added the Team:Platforms Label for the Integrations - Platforms team label Aug 17, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/integrations-platforms (Team:Platforms)

@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Aug 17, 2020
@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: [Pull request #20627 opened]

  • Start Time: 2020-08-17T08:00:35.841+0000

  • Duration: 73 min 2 sec

Test stats 🧪

Test Results
Failed 0
Passed 18002
Skipped 1827
Total 19829

@jarpy jarpy requested a review from ChrsMark August 17, 2020 23:07
@jarpy
Copy link
Contributor Author

jarpy commented Aug 17, 2020

I originally requested @blakerouse for review, but it looks like Blake is taking a quick break.

Would you mind taking a look, @ChrsMark? Thanks.

Note: The Travis test failure is for the test tests/system/test_unix.py::Test::test_unix_with_newline_delimiter on OSX. It's failing with OSError: [Errno 24] Too many open files, which seems to be happening on several different branches.

@jarpy
Copy link
Contributor Author

jarpy commented Aug 24, 2020

Looks like @ChrsMark is now on vacation, but @blakerouse is back! :D

Copy link
Contributor

@blakerouse blakerouse left a comment

Choose a reason for hiding this comment

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

Change looks good to me, thanks for the fix!

@jarpy
Copy link
Contributor Author

jarpy commented Aug 26, 2020

Thanks! I'm not sure about your team's merge/backport procedure. Could you help? Cheers.

@jarpy
Copy link
Contributor Author

jarpy commented Sep 1, 2020

What are the next steps for getting this merged? Thanks.

@blakerouse blakerouse merged commit 345e045 into master Sep 1, 2020
@blakerouse blakerouse deleted the rfc1123-kind-cluster-name branch September 1, 2020 11:53
blakerouse pushed a commit to blakerouse/beats that referenced this pull request Sep 1, 2020
When creating a kind cluster, we must use a cluster name that is a valid
Kubernetes resource name, and by extension, a valid DNS name. If not,
kind cluster provisioning can fail when kind tries to create resources
with invalid names.

For example, if trying to create a cluster called "a_b" (underscores
are not permitted), control-plane provisioning will fail with:

  host 'a_b-control-plane' must be [...] a valid RFC-1123 DNS subdomain

(cherry picked from commit 345e045)
@blakerouse
Copy link
Contributor

@jarpy Sorry, forgot about merging this. Merged it and created the backport PR for 7.x.

@jarpy
Copy link
Contributor Author

jarpy commented Sep 1, 2020

No worries. Many thanks!

v1v added a commit to v1v/beats that referenced this pull request Sep 2, 2020
…ne-2.0

* upstream/master: (87 commits)
  [packaging] Normalise GCP bucket folder structure (elastic#20903)
  [Metricbeat] Add billing metricset into googlecloud module (elastic#20812)
  Include python docs in devguide index (elastic#20917)
  Avoid generating incomplete configurations in autodiscover (elastic#20898)
  Improve docs of leaderelection configuration (elastic#20601)
  Document how to set the ES host and Kibana URLs in Ingest Manager (elastic#20874)
  docs: Update beats for APM (elastic#20881)
  Adding cborbeat to community beats (elastic#20884)
  Bump kibana version to 7.9.0 in x-pack/metricbeat (elastic#20899)
  Kubernetes state_daemonset metricset for Metricbeat (elastic#20649)
  [Filebeat][zeek] Add new x509 fields to zeek (elastic#20867)
  [Filebeat][Gsuite] Add note about admin in gsuite docs (elastic#20855)
  Ensure kind cluster has RFC1123 compliant name (elastic#20627)
  Setup python paths in test runner configuration (elastic#20832)
  docs: Add  `processor.event` info to Logstash output (elastic#20721)
  docs: update cipher suites (elastic#20697)
  [ECS] Update ecs to 1.6.0 (elastic#20792)
  Fix path in hits docs (elastic#20447)
  Update filebeat azure module documentation (elastic#20815)
  Remove duplicate ListGroupsForUsers in aws/cloudtrail (elastic#20788)
  ...
blakerouse added a commit that referenced this pull request Sep 22, 2020
When creating a kind cluster, we must use a cluster name that is a valid
Kubernetes resource name, and by extension, a valid DNS name. If not,
kind cluster provisioning can fail when kind tries to create resources
with invalid names.

For example, if trying to create a cluster called "a_b" (underscores
are not permitted), control-plane provisioning will fail with:

  host 'a_b-control-plane' must be [...] a valid RFC-1123 DNS subdomain

(cherry picked from commit 345e045)

Co-authored-by: Toby McLaughlin <toby@jarpy.net>
melchiormoulin pushed a commit to melchiormoulin/beats that referenced this pull request Oct 14, 2020
When creating a kind cluster, we must use a cluster name that is a valid
Kubernetes resource name, and by extension, a valid DNS name. If not,
kind cluster provisioning can fail when kind tries to create resources
with invalid names.

For example, if trying to create a cluster called "a_b" (underscores
are not permitted), control-plane provisioning will fail with:

  host 'a_b-control-plane' must be [...] a valid RFC-1123 DNS subdomain
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Team:Platforms Label for the Integrations - Platforms team v7.10.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants