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

Re-Added time synchronization between host/VM #4991

Merged
merged 1 commit into from
Aug 14, 2019
Merged

Re-Added time synchronization between host/VM #4991

merged 1 commit into from
Aug 14, 2019

Conversation

reegnz
Copy link
Contributor

@reegnz reegnz commented Aug 6, 2019

This commit attempts to add back the missing time synchronization feature
to Minikube that was removed earlier with #3476.

As mentioned in #1378 we have an alternative solution for time
synchronization for Oracle VirtualBox, so there we don't want to enable
systemd-timesyncd.

We are using systemd conditional activation on systemd-timesyncd and
exclude systems that have an oracle hypervisor hosting the vm (currently
that's virtualbox for our purposes).

Fixes #1378

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. do-not-merge/invalid-commit-message Indicates that a PR should not merge because it has an invalid commit message. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Aug 6, 2019
@k8s-ci-robot
Copy link
Contributor

Hi @reegnz. 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.

@k8s-ci-robot 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 Aug 6, 2019
@minikube-bot
Copy link
Collaborator

Can one of the admins verify this patch?

@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Aug 6, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: reegnz
To complete the pull request process, please assign ra489
You can assign the PR to them by writing /assign @ra489 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:

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

@reegnz
Copy link
Contributor Author

reegnz commented Aug 6, 2019

I didn't manage to rebuild the ISO and test with it, maybe someone could help out with that though.

@reegnz
Copy link
Contributor Author

reegnz commented Aug 6, 2019

Does the CI build the ISO? In that case it would make it much more easy for me to test it out. On my machine I built the ISO for more than an hour when I gave up on it.

@afbjorklund
Copy link
Collaborator

Looks reasonable, never understood why systemd spells virtualbox as "oracle"

@reegnz reegnz changed the title WIP: Reenable systemd-timesyncd on hypervisors except oracle WIP: Reenable systemd-timesyncd on hypervisors except virtualbox Aug 6, 2019
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/invalid-commit-message Indicates that a PR should not merge because it has an invalid commit message. label Aug 7, 2019
@reegnz
Copy link
Contributor Author

reegnz commented Aug 7, 2019

OOOKAY, I waited for the full iso build to finish now (pretty brutal time-wise).

I found a mistake I made with the drop-in folder name that I fixed (systemd-timesync.d -> systemd-timesyncd.service.d).

Running in virtualbox, displaying the drop-in and the condition failed text is what we want here. The timedatectl command output is a bit confusing though, but the key there is 'System clock synchronized': no:

➜ minikube ssh
                         _             _
            _         _ ( )           ( )
  ___ ___  (_)  ___  (_)| |/')  _   _ | |_      __
/' _ ` _ `\| |/' _ `\| || , <  ( ) ( )| '_`\  /'__`\
| ( ) ( ) || || ( ) || || |\`\ | (_) || |_) )(  ___/
(_) (_) (_)(_)(_) (_)(_)(_) (_)`\___/'(_,__/'`\____)

$ systemctl status systemd-timesyncd
● systemd-timesyncd.service - Network Time Synchronization
   Loaded: loaded (/usr/lib/systemd/system/systemd-timesyncd.service; enabled; vendor preset: enabled)
  Drop-In: /etc/systemd/system/systemd-timesyncd.service.d
           └─disable-virtualbox.conf
   Active: inactive (dead)
Condition: start condition failed at Wed 2019-08-07 09:20:31 UTC; 43s ago
     Docs: man:systemd-timesyncd.service(8)
$ timedatectl
                      Local time: Wed 2019-08-07 09:21:19 UTC
                  Universal time: Wed 2019-08-07 09:21:19 UTC
                        RTC time: Wed 2019-08-07 09:21:18
                       Time zone: Etc/UTC (UTC, +0000)
       System clock synchronized: no
systemd-timesyncd.service active: yes
                 RTC in local TZ: no
$

Running in a different hypervisor, for me it was hyperkit. Displays drop-in file and unit activates properly, timedatectl shows expected output:

➜ minikube ssh
                         _             _
            _         _ ( )           ( )
  ___ ___  (_)  ___  (_)| |/')  _   _ | |_      __
/' _ ` _ `\| |/' _ `\| || , <  ( ) ( )| '_`\  /'__`\
| ( ) ( ) || || ( ) || || |\`\ | (_) || |_) )(  ___/
(_) (_) (_)(_)(_) (_)(_)(_) (_)`\___/'(_,__/'`\____)

$ systemctl status systemd-timesyncd
● systemd-timesyncd.service - Network Time Synchronization
   Loaded: loaded (/usr/lib/systemd/system/systemd-timesyncd.service; enabled; vendor preset: enabled)
  Drop-In: /etc/systemd/system/systemd-timesyncd.service.d
           └─disable-virtualbox.conf
   Active: active (running) since Wed 2019-08-07 09:23:05 UTC; 1min 59s ago
     Docs: man:systemd-timesyncd.service(8)
 Main PID: 1609 (systemd-timesyn)
   Status: "Synchronized to time server 216.239.35.8:123 (time3.google.com)."
    Tasks: 2 (limit: 4915)
   CGroup: /system.slice/systemd-timesyncd.service
           └─1609 /usr/lib/systemd/systemd-timesyncd

Aug 07 09:24:07 minikube systemd-timesyncd[1609]: Network configuration changed, trying to establish connection.
Aug 07 09:24:07 minikube systemd-timesyncd[1609]: Synchronized to time server 216.239.35.8:123 (time3.google.com).
Aug 07 09:24:07 minikube systemd-timesyncd[1609]: Network configuration changed, trying to establish connection.
Aug 07 09:24:07 minikube systemd-timesyncd[1609]: Synchronized to time server 216.239.35.8:123 (time3.google.com).
Aug 07 09:24:07 minikube systemd-timesyncd[1609]: Network configuration changed, trying to establish connection.
Aug 07 09:24:07 minikube systemd-timesyncd[1609]: Synchronized to time server 216.239.35.8:123 (time3.google.com).
Aug 07 09:24:07 minikube systemd-timesyncd[1609]: Network configuration changed, trying to establish connection.
Aug 07 09:24:07 minikube systemd-timesyncd[1609]: Synchronized to time server 216.239.35.8:123 (time3.google.com).
Aug 07 09:24:09 minikube systemd-timesyncd[1609]: Network configuration changed, trying to establish connection.
Aug 07 09:24:09 minikube systemd-timesyncd[1609]: Synchronized to time server 216.239.35.8:123 (time3.google.com).
$ timedatectl
                      Local time: Wed 2019-08-07 09:25:12 UTC
                  Universal time: Wed 2019-08-07 09:25:12 UTC
                        RTC time: Wed 2019-08-07 09:25:12
                       Time zone: Etc/UTC (UTC, +0000)
       System clock synchronized: yes
systemd-timesyncd.service active: yes
                 RTC in local TZ: no

Seems like a success to me. @afbjorklund what do you think?

@reegnz reegnz changed the title WIP: Reenable systemd-timesyncd on hypervisors except virtualbox Reenable systemd-timesyncd on hypervisors except virtualbox Aug 7, 2019
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 7, 2019
This commit attempts to add back the missing time synchronization feature
to Minikube that was removed earlier with #3476.

As mentioned in #1378 we have an alternative solution for time
synchronization for Oracle VirtualBox, so there we don't want to enable
systemd-timesyncd.

We are using systemd conditional activation on systemd-timesyncd and
exclude systems that have an oracle hypervisor hosting the vm (currently
that's virtualbox for our purposes).
@reegnz reegnz changed the title Reenable systemd-timesyncd on hypervisors except virtualbox Reenable systemd-timesyncd, except virtualbox Aug 7, 2019
@RA489
Copy link

RA489 commented Aug 8, 2019

@minikube-bot OK to test

@medyagh medyagh changed the title Reenable systemd-timesyncd, except virtualbox Re-Added time synchronization between host/VM Aug 14, 2019
@medyagh
Copy link
Member

medyagh commented Aug 14, 2019

@reegnz Thank you so much for creating this issue ! this fixes one of the complains we had for minikube and I believe closes many issues !

thank you so much

@medyagh medyagh merged commit 25e057c into kubernetes:master Aug 14, 2019
@reegnz
Copy link
Contributor Author

reegnz commented Aug 19, 2019

@medyagh Good to know this is such an important contribution. :) Always glad to help.

@reegnz reegnz deleted the bugfix/reenable-systemd-timesyncd branch August 26, 2019 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

hyperkit clock falls behind: failed to write or validate certificate: the certificate is not valid yet
6 participants