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.

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

timezone module: allow suse linux as target #36719

Merged
merged 2 commits into from
Mar 15, 2018
Merged

timezone module: allow suse linux as target #36719

merged 2 commits into from
Mar 15, 2018

Conversation

tmshn
Copy link
Contributor

@tmshn tmshn commented Feb 26, 2018

SUMMARY

SUSE Linux uses /etc/sysconfig/clock for timezone config like CentOS do, but it's format is differnt: the key is TIMEZONE rather than ZONE.

To make this module work for SUSE, I added a if statement to decide appropriate key.

Note: this is reported in and will fixes #36237

Oh, and actually pointed out in my very original pull request too...!
ansible/ansible-modules-extras#2414 (comment)

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME
  • timezone module
ANSIBLE VERSION
ansible 2.6.0 (timezone#32048 9f4ebbc25d) last updated 2018/02/26 17:36:07 (GMT +900)
  config file = None
  configured module search path = [u'/Users/xxxxx/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /Users/xxxxx/Documents/dev/oss/ansible/lib/ansible
  executable location = ./bin/ansible
  python version = 2.7.12 |Anaconda custom (x86_64)| (default, Jul  2 2016, 17:43:17) [GCC 4.2.1 (Based on Apple Inc. build 5658) (LLVM build 2336.11.00)]

@ansibot
Copy link
Contributor

ansibot commented Feb 26, 2018

cc @indrajitr @jasperla
click here for bot help

@ansibot ansibot added bugfix_pull_request community_review In order to be merged, this PR must follow the community review workflow. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. owner_pr This PR is made by the module's maintainer. support:community This issue/PR relates to code supported by the Ansible community. labels Feb 26, 2018
@tmshn tmshn mentioned this pull request Feb 26, 2018
Closed
@Akasurde Akasurde removed the needs_triage Needs a first human triage before being processed. label Feb 26, 2018
Akasurde
Akasurde reviewed Feb 26, 2018
View reviewed changes
lib/ansible/modules/system/timezone.py Outdated
# The key for timezone might be `ZONE` or `TIMEZONE`
# (the former is used in RHEL/CentOS and the latter is used in SUSE linux).
# So check the content of /etc/sysconfig/clock and decide which key to use.
file = open(self.conf_files['name'], mode='r')
Copy link
Member

Choose a reason for hiding this comment

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

Could you please replace this with

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I didn't know that Ansible abandoned python 2.4...cool!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Modified: 0d5f2ac

@ahotzler
Copy link

I successfully tested 610c325 and 0d5f2ac on SLES11SP4

@Akasurde
Copy link
Member

Look good to me.

@indrajitr
Copy link
Contributor

👍 Looks good to me

@ansibot ansibot added bug This issue/PR relates to a bug. and removed bugfix_pull_request labels Mar 2, 2018
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Mar 10, 2018
@Jellyfrog
Copy link

This also applies to SLES11SP3

@tmshn
Copy link
Contributor Author

tmshn commented Mar 15, 2018

What can I do for this to get merged?

@indrajitr can you say "shipit" instead of "looks good to me", plz? maybe that invokes ansibot

mkrizek
mkrizek approved these changes Mar 15, 2018
View reviewed changes
Copy link
Contributor

@mkrizek mkrizek left a comment

Choose a reason for hiding this comment

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

👍

@mkrizek
Copy link
Contributor

mkrizek commented Mar 15, 2018

rebuild_merge

@ansibot ansibot removed the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Mar 15, 2018
@ansibot ansibot merged commit 73512ee into ansible:devel Mar 15, 2018
@ansible ansible locked and limited conversation to collaborators Apr 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Akasurde Akasurde Akasurde left review comments

@mkrizek mkrizek mkrizek approved these changes

Assignees
No one assigned
Labels
bug This issue/PR relates to a bug. community_review In order to be merged, this PR must follow the community review workflow. module This issue/PR relates to a module. owner_pr This PR is made by the module's maintainer. support:community This issue/PR relates to code supported by the Ansible community.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

module timezone doesnt work with SLES11SP4
7 participants
@tmshn @ansibot @ahotzler @Akasurde @indrajitr @Jellyfrog @mkrizek