Skip to content

template: separate kubelet config into new MachineConfig #213

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.

Sign up for GitHub

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

Jump to bottom
Merged
openshift-merge-robot merged 2 commits into from
Dec 11, 2018
Merged

template: separate kubelet config into new MachineConfig #213

openshift-merge-robot merged 2 commits into from
Dec 11, 2018

Conversation

@sjenning
Copy link
Contributor

@sjenning sjenning commented Dec 4, 2018

@abhinavdahiya @derekwaynecarr @gnufied @ashcrow

Move the /etc/kubernetes/kubelet.conf and /etc/systemd/system/kubelet.service to a separate MachineConfig

@openshift-ci-robot openshift-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Dec 4, 2018
@sjenning
Copy link
Contributor Author

sjenning commented Dec 4, 2018

Unit tests currently assume one MachineConfig per role as well. They'll need to be reworked. I tested this locally and it works. I would expect e2e-aws to pass.

@rphillips
Copy link
Contributor

rphillips commented Dec 6, 2018 

 got error on docker inspect fork/exec /bin/docker: no such file or directory

/retest

@sjenning
Copy link
Contributor Author

sjenning commented Dec 7, 2018

/retest

@abhinavdahiya
Copy link
Contributor

abhinavdahiya commented Dec 7, 2018

/approve

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 7, 2018
@abhinavdahiya
Copy link
Contributor

abhinavdahiya commented Dec 7, 2018

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 7, 2018
@openshift-ci-robot openshift-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed lgtm Indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Dec 7, 2018
@abhinavdahiya
Copy link
Contributor

abhinavdahiya commented Dec 7, 2018

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 7, 2018
@ashcrow
Copy link
Member

ashcrow commented Dec 7, 2018

/test e2e-aws

1 similar comment
@rphillips
Copy link
Contributor

rphillips commented Dec 7, 2018

/test e2e-aws

@rphillips
Copy link
Contributor

rphillips commented Dec 9, 2018

/retest

@rphillips
Copy link
Contributor

rphillips commented Dec 10, 2018

/test e2e-aws

@rphillips
Copy link
Contributor

rphillips commented Dec 10, 2018

@abhinavdahiya Do you know why this error might pop up?

CONFLICT (rename/delete): pkg/controller/template/test_data/templates/openstack/worker/files/-etc-hosts deleted in HEAD and renamed to pkg/controller/template/test_data/templates/worker/00-worker/openstack/files/-etc-hosts in 3ad4975b0b392073a6fa5dc63be9591a21114fc2. Version 3ad4975b0b392073a6fa5dc63be9591a21114fc2 of pkg/controller/template/test_data/templates/worker/00-worker/openstack/files/-etc-hosts left in tree.
CONFLICT (rename/delete): pkg/controller/template/test_data/templates/openstack/worker/files/-etc-docker-certs.d-docker-registry.default.svc:5000-ca.crt deleted in HEAD and renamed to pkg/controller/template/test_data/templates/worker/00-worker/openstack/files/-etc-docker-certs.d-docker-registry.default.svc:5000-ca.crt in 3ad4975b0b392073a6fa5dc63be9591a21114fc2. Version 3ad4975b0b392073a6fa5dc63be9591a21114fc2 of pkg/controller/template/test_data/templates/worker/00-worker/openstack/files/-etc-docker-certs.d-docker-registry.default.svc:5000-ca.crt left in tree.
CONFLICT (rename/delete): pkg/controller/template/test_data/templates/openstack/master/files/-etc-hosts deleted in HEAD and renamed to pkg/controller/template/test_data/templates/worker/00-worker/libvirt/files/-etc-hosts in 3ad4975b0b392073a6fa5dc63be9591a21114fc2. Version 3ad4975b0b392073a6fa5dc63be9591a21114fc2 of pkg/controller/template/test_data/templates/worker/00-worker/libvirt/files/-etc-hosts left in tree.
CONFLICT (rename/delete): pkg/controller/template/test_data/templates/openstack/master/files/-etc-docker-certs.d-docker-registry.default.svc:5000-ca.crt deleted in HEAD and renamed to pkg/controller/template/test_data/templates/worker/00-worker/libvirt/files/-etc-docker-certs.d-docker-registry.default.svc:5000-ca.crt in 3ad4975b0b392073a6fa5dc63be9591a21114fc2. Version 3ad4975b0b392073a6fa5dc63be9591a21114fc2 of pkg/controller/template/test_data/templates/worker/00-worker/libvirt/files/-etc-docker-certs.d-docker-registry.default.svc:5000-ca.crt left in tree.

@abhinavdahiya
Copy link
Contributor

abhinavdahiya commented Dec 10, 2018

#225 removed some files.

@rphillips
Copy link
Contributor

rphillips commented Dec 10, 2018

I'll rebase. Thanks!

@rphillips
Copy link
Contributor

rphillips commented Dec 10, 2018

@sjenning If you reset your PR to https://github.com/rphillips/machine-config-operator/tree/add-kubelet-mcfg it'll fix the conflicts. (I don't have permission to push to your PR).

@openshift-ci-robot openshift-ci-robot removed lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Dec 10, 2018
@openshift-ci-robot openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Dec 10, 2018
@sjenning
Copy link
Contributor Author

sjenning commented Dec 10, 2018

rebased, see how it goes

@sjenning
Copy link
Contributor Author

sjenning commented Dec 11, 2018

/retest

@abhinavdahiya
Copy link
Contributor

abhinavdahiya commented Dec 11, 2018

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 11, 2018
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Dec 11, 2018

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinavdahiya, sjenning

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@openshift-merge-robot openshift-merge-robot merged commit d723f6f into openshift:master Dec 11, 2018
wking
wking reviewed Feb 28, 2019
View reviewed changes
pkg/controller/template/test_data/templates/master/01-master-kubelet/aws/units/kubelet.service
--minimum-container-ttl-duration=6m0s \
--client-ca-file=/etc/kubernetes/ca.crt \
--cloud-provider=aws \
\
Copy link
Member

@wking wking Feb 28, 2019

Choose a reason for hiding this comment

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

Why this empty line? (spun off from here)

osherdp pushed a commit to osherdp/machine-config-operator that referenced this pull request Apr 13, 2021
@openshift-merge-robot
Merge pull request openshift#213 from gabemontero/php-eol
6868be5 
Bug 1782467: bump samples since PHP EOL template updates now breaking image-eco
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wking wking wking left review comments

@crawford crawford Awaiting requested review from crawford

@jlebon jlebon Awaiting requested review from jlebon

Assignees

@abhinavdahiya abhinavdahiya

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@sjenning @rphillips @abhinavdahiya @ashcrow @openshift-ci-robot @wking @openshift-merge-robot