Removed default hostname-override #10595

DOboznyi · 2021-01-16T14:05:03Z

There's no sense in hostname-override because it's not working when cloud-provider flag set.

Signed-off-by: Dmytro Oboznyi dmytro.oboznyi@syncier.com

k8s-ci-robot · 2021-01-16T14:05:11Z

Hi @DOboznyi. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

olemarkus · 2021-01-16T14:11:13Z

/ok-to-test

DOboznyi · 2021-01-16T20:24:11Z

/retest

DOboznyi · 2021-01-16T20:50:46Z

@olemarkus why such tests failing? How can I fix them?

olemarkus · 2021-01-17T06:42:51Z

Run ./hack/update-expected

DOboznyi · 2021-01-17T17:29:00Z

/hack/update-expected

olemarkus · 2021-01-17T17:30:59Z

It's a script found in the git repos

DOboznyi · 2021-01-17T18:03:19Z

/retest

k8s-ci-robot · 2021-02-23T11:00:35Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: HELLRAZOR6666
To complete the pull request process, please assign mikesplain after the PR has been reviewed.
You can assign the PR to them by writing /assign @mikesplain in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

DOboznyi · 2021-02-23T14:40:35Z

@olemarkus CI tests removed, could you help with e2e failures?

olemarkus · 2021-02-23T18:57:53Z

The cert is wrong. You see the error:

I0223 12:18:32.931137    4277 kubelet_node_status.go:71] Attempting to register node ip-172-20-59-153.eu-west-1.compute.internal
E0223 12:18:32.933453    4277 kubelet_node_status.go:93] Unable to register node "ip-172-20-59-153.eu-west-1.compute.internal" with API server: nodes "ip-172-20-59-153.eu-west-1.compute.internal" is forbidden: node "ip-172-20-59-153" is not allowed to modify node "ip-172-20-59-153.eu-west-1.compute.internal"

Further down there is

I0223 12:18:33.336972    4277 csi_plugin.go:1016] Failed to contact API server when waiting for CSINode publishing: csinodes.storage.k8s.io "ip-172-20-59-153.eu-west-1.compute.internal" is forbidden: User "system:node:ip-172-20-59-153" cannot get resource "csinodes" in API group "storage.k8s.io" at the cluster scope: can only access CSINode with the same name as the requesting node

Use r system:node:ip-172-20-59-153 should be `system:node:ip-172-20-59-153..eu-west-1.compute.internal"

olemarkus · 2021-02-23T18:59:30Z

nodeup/pkg/model/context.go

@@ -484,79 +481,16 @@ func (c *NodeupModelContext) BuildPrivateKeyTask(ctx *fi.ModelBuilderContext, na
 // NodeName returns the name of the local Node, as it will be created in k8s
 func (c *NodeupModelContext) NodeName() (string, error) {


This function need to return FQDN as it is used for the node cert name.See my previous comment.

Okay, i'll return code which return FQDN, thanks

DOboznyi · 2021-02-25T13:06:28Z

@olemarkus seems as previous problem gone. Are the other tests critical?

DOboznyi · 2021-02-25T13:28:46Z

/retest

olemarkus · 2021-02-26T07:53:13Z

/retest

olemarkus · 2021-02-26T07:56:34Z

nodeup/pkg/model/context.go

-	if c.IsMaster && c.Cluster.Spec.MasterKubelet.HostnameOverride != "" {
-		hostnameOverride = c.Cluster.Spec.MasterKubelet.HostnameOverride
-	}
+	hostnameOverride := string('@') + c.Cluster.Spec.CloudProvider


I assume this is just temporary for testing?

How should i change this part?

Isn't the goal of completely getting rid of hostnameOverride?

olemarkus · 2021-02-26T07:57:21Z

pkg/model/components/kubelet.go

@@ -208,6 +195,9 @@ func (b *KubeletOptionsBuilder) BuildOptions(o interface{}) error {
 		if _, found := clusterSpec.Kubelet.FeatureGates["CSIMigrationAWS"]; !found {
 			clusterSpec.Kubelet.FeatureGates["CSIMigrationAWS"] = "true"
 		}
+		if _, found := clusterSpec.Kubelet.FeatureGates["CSIMigrationAWSComplete"]; !found {
+			clusterSpec.Kubelet.FeatureGates["CSIMigrationAWSComplete"] = "true"
+		}


Can you rebase this against master? If not you may get into conflicts.

Fixed tests Fix evaluateHostnameOverride Deprecated hostnameOverride for kubelet Removed hostname-override for kube-proxy Changed hostname to actual value Remove EvaluateHostnameOverride Update test Update bazel Removed tests Signed-off-by: Dmytro Oboznyi <dmytro.oboznyi@syncier.com>

Signed-off-by: Dmytro Oboznyi <dmytro.oboznyi@syncier.com>

olemarkus · 2021-03-02T08:38:50Z

nodeup/pkg/model/context.go

-	if c.IsMaster && c.Cluster.Spec.MasterKubelet.HostnameOverride != "" {
-		hostnameOverride = c.Cluster.Spec.MasterKubelet.HostnameOverride
-	}
+	hostnameOverride := string('@') + c.Cluster.Spec.CloudProvider

 	nodeName, err := EvaluateHostnameOverride(hostnameOverride)


Is this now the only call in the code going to EvaluateHostnameOverride? if so, I think you can inline the function here. NodeName is a much more appropriate name than EvaluateHostnameOverride for what this function is used for now.

wdyt?

k8s-ci-robot · 2021-04-06T16:13:05Z

@DOboznyi: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

hakman · 2021-05-21T07:35:20Z

Just wanted to leave a comment here that this may have gone against what upstream was thinking.
Seems that the --cloud-provider flag will be removed soon, as discussed in kubernetes/kubernetes#90408.

k8s-triage-robot · 2021-08-19T08:01:10Z

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

Mark this issue or PR as fresh with /remove-lifecycle stale
Mark this issue or PR as rotten with /lifecycle rotten
Close this issue or PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot · 2021-09-18T08:39:00Z

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

Mark this issue or PR as fresh with /remove-lifecycle rotten
Close this issue or PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

k8s-ci-robot · 2021-10-02T13:26:51Z

@DOboznyi: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-kops-e2e-cni-calico	`ec9de48`	link	`/test pull-kops-e2e-cni-calico`
pull-kops-e2e-cni-weave	`ec9de48`	link	`/test pull-kops-e2e-cni-weave`
pull-kops-e2e-k8s-containerd	`e2b9f47`	link	`/test pull-kops-e2e-k8s-containerd`
pull-kops-e2e-kubernetes-aws	`e2b9f47`	link	`/test pull-kops-e2e-kubernetes-aws`
pull-kops-e2e-cni-amazonvpc	`e2b9f47`	link	`/test pull-kops-e2e-cni-amazonvpc`
pull-kops-e2e-cni-flannel	`e2b9f47`	link	true	`/test pull-kops-e2e-cni-flannel`
pull-kops-e2e-cni-canal	`e2b9f47`	link	true	`/test pull-kops-e2e-cni-canal`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

k8s-triage-robot · 2021-11-01T14:16:09Z

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

Reopen this issue or PR with /reopen
Mark this issue or PR as fresh with /remove-lifecycle rotten
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

k8s-ci-robot · 2021-11-01T14:16:29Z

@k8s-triage-robot: Closed this PR.

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied

After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied

After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

Reopen this issue or PR with /reopen

Mark this issue or PR as fresh with /remove-lifecycle rotten

Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jan 16, 2021

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jan 16, 2021

k8s-ci-robot requested review from rdrgmnzs and zetaab January 16, 2021 14:05

k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jan 16, 2021

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jan 16, 2021

k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jan 17, 2021

DOboznyi force-pushed the Remove-default-hostname-override branch from ba40cc8 to 228493b Compare January 17, 2021 18:18

k8s-ci-robot added do-not-merge/contains-merge-commits and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jan 17, 2021

DOboznyi force-pushed the Remove-default-hostname-override branch from 228493b to 4a1ec5c Compare January 17, 2021 19:04

k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed do-not-merge/contains-merge-commits size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 17, 2021

DOboznyi force-pushed the Remove-default-hostname-override branch from 4a1ec5c to afc5b57 Compare January 17, 2021 19:17

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jan 17, 2021

DOboznyi force-pushed the Remove-default-hostname-override branch 2 times, most recently from e1aa07a to 1ee1138 Compare January 17, 2021 19:21

DOboznyi force-pushed the Remove-default-hostname-override branch from a160531 to a073a0d Compare February 23, 2021 11:52

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 23, 2021

DOboznyi force-pushed the Remove-default-hostname-override branch from a073a0d to 8243566 Compare February 23, 2021 12:09

olemarkus reviewed Feb 23, 2021

View reviewed changes

olemarkus reviewed Feb 26, 2021

View reviewed changes

Dmytro Oboznyi added 2 commits March 1, 2021 12:13

Reverted changes in context.go

0aa1be1

Signed-off-by: Dmytro Oboznyi <dmytro.oboznyi@syncier.com>

DOboznyi force-pushed the Remove-default-hostname-override branch from e1d06b7 to 0aa1be1 Compare March 1, 2021 10:13

Fix

e2b9f47

Signed-off-by: Dmytro Oboznyi <dmytro.oboznyi@syncier.com>

olemarkus reviewed Mar 2, 2021

View reviewed changes

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 6, 2021

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Aug 19, 2021

olemarkus mentioned this pull request Sep 1, 2021

Avoid setting hostname-override #12245

Closed

k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Sep 18, 2021

k8s-ci-robot closed this Nov 1, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Removed default hostname-override #10595

Removed default hostname-override #10595

DOboznyi commented Jan 16, 2021

k8s-ci-robot commented Jan 16, 2021

olemarkus commented Jan 16, 2021

DOboznyi commented Jan 16, 2021

DOboznyi commented Jan 16, 2021

olemarkus commented Jan 17, 2021

DOboznyi commented Jan 17, 2021

olemarkus commented Jan 17, 2021

DOboznyi commented Jan 17, 2021

k8s-ci-robot commented Feb 23, 2021

DOboznyi commented Feb 23, 2021 •

edited

Loading

olemarkus commented Feb 23, 2021

olemarkus Feb 23, 2021

DOboznyi Feb 24, 2021 •

edited

Loading

DOboznyi commented Feb 25, 2021

DOboznyi commented Feb 25, 2021

olemarkus commented Feb 26, 2021

olemarkus Feb 26, 2021

DOboznyi Mar 1, 2021

olemarkus Mar 2, 2021

olemarkus Feb 26, 2021

DOboznyi Mar 1, 2021

olemarkus Mar 2, 2021

k8s-ci-robot commented Apr 6, 2021

hakman commented May 21, 2021

k8s-triage-robot commented Aug 19, 2021

k8s-triage-robot commented Sep 18, 2021

k8s-ci-robot commented Oct 2, 2021

k8s-triage-robot commented Nov 1, 2021

k8s-ci-robot commented Nov 1, 2021

		@@ -484,79 +481,16 @@ func (c NodeupModelContext) BuildPrivateKeyTask(ctx fi.ModelBuilderContext, na
		// NodeName returns the name of the local Node, as it will be created in k8s
		func (c *NodeupModelContext) NodeName() (string, error) {

Removed default hostname-override #10595

Removed default hostname-override #10595

Conversation

DOboznyi commented Jan 16, 2021

k8s-ci-robot commented Jan 16, 2021

olemarkus commented Jan 16, 2021

DOboznyi commented Jan 16, 2021

DOboznyi commented Jan 16, 2021

olemarkus commented Jan 17, 2021

DOboznyi commented Jan 17, 2021

olemarkus commented Jan 17, 2021

DOboznyi commented Jan 17, 2021

k8s-ci-robot commented Feb 23, 2021

DOboznyi commented Feb 23, 2021 • edited Loading

olemarkus commented Feb 23, 2021

olemarkus Feb 23, 2021

Choose a reason for hiding this comment

DOboznyi Feb 24, 2021 • edited Loading

Choose a reason for hiding this comment

DOboznyi commented Feb 25, 2021

DOboznyi commented Feb 25, 2021

olemarkus commented Feb 26, 2021

olemarkus Feb 26, 2021

Choose a reason for hiding this comment

DOboznyi Mar 1, 2021

Choose a reason for hiding this comment

olemarkus Mar 2, 2021

Choose a reason for hiding this comment

olemarkus Feb 26, 2021

Choose a reason for hiding this comment

DOboznyi Mar 1, 2021

Choose a reason for hiding this comment

olemarkus Mar 2, 2021

Choose a reason for hiding this comment

k8s-ci-robot commented Apr 6, 2021

hakman commented May 21, 2021

k8s-triage-robot commented Aug 19, 2021

k8s-triage-robot commented Sep 18, 2021

k8s-ci-robot commented Oct 2, 2021

k8s-triage-robot commented Nov 1, 2021

k8s-ci-robot commented Nov 1, 2021

DOboznyi commented Feb 23, 2021 •

edited

Loading

DOboznyi Feb 24, 2021 •

edited

Loading