-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Bind the kubelet to the local ipv4 address #4417
Conversation
@liwenwu-amazon double checking with you that this was the recommended solution. /ok-to-test |
@chrislovecnm, I tested this based on our Slack conversation , and
Log from journalctl:
However, there is an issue pulling down the
Is this expected or am I doing something wrong? |
nodeup/pkg/model/kubelet.go
Outdated
if err != nil { | ||
glog.Fatalf("Couldn't fetch the local-ipv4 address from the ec2 meta-data: %v", err) | ||
} else { | ||
flags += " --node-ip=" + localIpv4 |
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 need the else as I think glog.Fatal will return an err. We may want to log and error and return err here.
@bksteiny no idea on the image, I would reach out to the AWS folks |
@chrislovecnm changed as requested |
You need to rebase and run that command as CI is failing. |
Looks good ... CI is not happy |
Code looks fine once CI is happy. Another option is to do this (though with |
@justinsb let me know if I should update the PR as you suggested, I don't have a preference |
Let’s just get CI fixed ;) |
You need to run the command to fix bazel. I mentioned it in a previous comment. |
@justinsb this needs to go into 1.9 since it is fixing a bug with the aws cni provider |
I pulled this PR locally and build a dev version of kop. I am NOT able to bring up a kop cluster successfully. Not sure it is my environment issue or this PR.
here is the output of kops validate error
The master instance is up already for over 10 minutes.
|
@dezmodue can we get really detailed instructions on how you tested? |
Hi, at the time I sent in the PR I had built kops and nodeup from the modified version in my repo:
Then I built 2 clusters from a yaml definition like the one in the issue #4218 -- they are still running fwiw
Today I have rebased on 034bad8 and ran again a test cluster:
The result is a healthy cluster as far as I can tell:
I logged into the node and checked the running kubelet process:
And the config:
I can see in the AWS console that the node and the masters have assigned secondary private IPs as expected.
I launched a pod and I can see it gets assigned an IP from the correct range:
I tore down the cluster now but if you have more questions please let me know. @chrislovecnm I have rebased and ran ./hack/update-bazel.sh - fwiw I am also available in the kops-users channel in slack |
/retest |
@dezmodue @justinsb @chrislovecnm Also, I have changed to use Gossip-based cluster. Not sure why Private DNS based cluster suddenly stopped working for me now. |
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
@dezmodue I think we want to cherry pick this into release-1.9 branch. Do you mind? |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: chrislovecnm, dezmodue The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Fine by me, anything I should do? |
@dezmodue just create a PR into the release branch, we are moving towards doing individual cherry picks |
#4568 -- hope this is correct |
No description provided.