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

Build config file for worker nodes too #224

Merged
merged 2 commits into from
Jan 15, 2019

Conversation

jorhett
Copy link
Contributor

@jorhett jorhett commented Dec 21, 2018

This resolves the same cloud_provider problem for workers as the previous patch.
It also does some strategic structure for the new config-file-oriented world,
which will hopefully make it easier to obsolete older versions with less pain.

I wouldn't merge this yet, I don't think it's been tested all the ways it needs to be. We need some eyeballs on this in different environments.

@davejrt
Copy link
Contributor

davejrt commented Dec 21, 2018

I agree with the the changes you're proposing here, we've had internal discussions to do something similar with the use of config files for joining nodes with the newer versions of k8s.

There's a few things in here that are broken like the config files and the define types but generally speaking the direction is one I support

You're also welcome to grab me in the puppet community slack if that is an easier place to discuss any ideas you're having before putting a lot of time and effort into PR's etc.

@jorhett jorhett force-pushed the fix_node_join_with_cloud_provider branch 9 times, most recently from 33a21d9 to e1fd7e2 Compare December 24, 2018 23:26
@davejrt
Copy link
Contributor

davejrt commented Jan 4, 2019

Have been testing this branch over the xmas/new year period and can't seem to get a clean run with kream on the worker nodes first go. I need to re run puppet, as it seems to skip over the unless statement the first time and not put any of the worker config down.

Are you only working in AWS?

@jorhett jorhett force-pushed the fix_node_join_with_cloud_provider branch from 60d4494 to 513f9d6 Compare January 4, 2019 16:23
@jorhett
Copy link
Contributor Author

jorhett commented Jan 4, 2019

Are you talking about this unless? 513f9d6#diff-60ae41fd0a31977447947f59940ee9a4R444

I tested this in Kream with 1.10.2, 1.11.2, 1.12.3, 1.13.0 and 1.13.1 and I'm not seeing that problem. What does your test data look like?

@davejrt
Copy link
Contributor

davejrt commented Jan 4, 2019

That is the unless I'm referring to. Appears the error only occurs on ubuntu. I'm running it with the default data in kream and pointing my puppetfile to your fork. Here are some excerpts from the log.

==> kube-node-02: Debug: /Stage[main]/Kubernetes::Repos/Apt::Source[docker]/Apt::Setting[list-docker]/File[/etc/apt/sources.list.d/docker.list]: Adding autorequire relationship with File[sources.list.d] ==> kube-node-02: Error: Could not set 'file' on ensure: No such file or directory @ dir_s_mkdir - /etc/kubernetes/config.yaml20190104-13584-1x66zhn.lock (file: /tmp/modules/kubernetes/manifests/config/worker.pp, line: 40) ==> kube-node-02: Error: Could not set 'file' on ensure: No such file or directory @ dir_s_mkdir - /etc/kubernetes/config.yaml20190104-13584-1x66zhn.lock (file: /tmp/modules/kubernetes/manifests/config/worker.pp, line: 40) ==> kube-node-02: Wrapped exception: ==> kube-node-02: No such file or directory @ dir_s_mkdir - /etc/kubernetes/config.yaml20190104-13584-1x66zhn.lock ==> kube-node-02: Error: /Stage[main]/Kubernetes::Config::Worker/File[/etc/kubernetes/config.yaml]/ensure: change from 'absent' to 'file' failed: Could not set 'file' on ensure: No such file or directory @ dir_s_mkdir - /etc/kubernetes/config.yaml20190104-13584-1x66zhn.lock (file: /tmp/modules/kubernetes/manifests/config/worker.pp, line: 40)

It skips over the unless statement and runs packages the services, without the worker config.

==> kube-node-02: Notice: /Stage[main]/Kubernetes::Packages/Sysctl[net.ipv4.ip_forward]/value: changed configuration value from '' to '1' ==> kube-node-02: Debug: Executing: '/sbin/sysctl -w net.ipv4.ip_forward=1' ==> kube-node-02: Debug: /Stage[main]/Kubernetes::Packages/Sysctl[net.ipv4.ip_forward]:The container Class[Kubernetes::Packages] will propagate my refresh event ==> kube-node-02: Debug: Class[Kubernetes::Packages]: The container Stage[main] will propagate my refresh event ==> kube-node-02: Debug: Class[Kubernetes::Packages]: The container Class[Kubernetes] will propagate my refresh event ==> kube-node-02: Debug: Class[Kubernetes::Service]: Resource is being skipped, unscheduling all events ==> kube-node-02: Notice: /Stage[main]/Kubernetes::Service/File[/etc/systemd/system/kubelet.service.d]: Dependency File[/etc/kubernetes/config.yaml] has failures: true ==> kube-node-02: Warning: /Stage[main]/Kubernetes::Service/File[/etc/systemd/system/kubelet.service.d]: Skipping because of failed dependencies

Jo Rhett added 2 commits January 6, 2019 23:12
This resolves the same cloud_provider problem for workers as the previous patch.
It also does some strategic structure for the new config-file-oriented world,
which will hopefully make it easier to obsolete older versions with less pain.
@jorhett jorhett force-pushed the fix_node_join_with_cloud_provider branch from 513f9d6 to c1c82b9 Compare January 7, 2019 07:18
@davejrt
Copy link
Contributor

davejrt commented Jan 8, 2019

I tested this on aws with ubuntu and it worked fine. Still can't get it to work with vagrant using puppet5/6 on debian and ubuntu...very odd.

@jorhett
Copy link
Contributor Author

jorhett commented Jan 10, 2019

I am gonna work on this, sorry, cycles have been hard to find last few days

@davejrt
Copy link
Contributor

davejrt commented Jan 10, 2019

@jorhett no problem. Just to be clear the PR is good, and we have every intention of merging once we can hopefully figure out this small issue. I've had someone else test to make sure it's not my local environment with the same result.

I am in the same boat in that I am working in the background whilst doing other things so let's post any results here and keep the conversation going

@davejrt
Copy link
Contributor

davejrt commented Jan 15, 2019

I'm going to merge this, and raise a separate issue to track this as a big in KREAM only, as a release is dependent on this code and master is currently broken without it. Thanks again for your contributions

@davejrt davejrt merged commit 48c7714 into puppetlabs:master Jan 15, 2019
@davejrt davejrt added this to the 3.2.0 milestone Jan 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants