New time period module #426

Max-checkmk · 2023-08-25T09:24:19Z

Pull request type

Please check the type of change your PR introduces:

What is the current behavior?

There is no module for time periods

What is the new behavior?

Added a module for creating, updating and deleting time periods

Other information

lgetwan

The documentation could be a bit more detailed. Probably, you could use the tasks from your integration test.

I tested the module with some use cases:

When creating a timeperiod that already exists, a failure is shown (should be "OK").
When updating a timeperiod without any actual change, the result is "changed" (should be "OK").
When adding an exclude time range, this isn't visible in the UI.
When deleting a timeperiod that doesn't exist, a failure is shown (should be "OK").

Here's the playbook i used:


# Make sure to keep the tests in sync!
# Rather use the tests than this playbook directly.

- name: "Test timeperiods"
  hosts: all
  gather_facts: 'no'
  vars_files:
    - ./vars/config.yml

  tasks:
  - name: create timeperiod
    checkmk.general.timeperiod:
      server_url: "{{ server_url }}"
      site: "{{ site }}"
      automation_user: "{{ automation_user }}"
      automation_secret: "{{ automation_secret }}"
      name: "lunch"
      alias: "Mahlzeit"
      active_time_ranges: [
              { 
                  "day": "all",
                  "time_ranges": [
                      {
                          "start": "12:00",
                          "end": "13:00"
                      }
                  ]
              }
          ]
      state: present
    delegate_to: localhost
    run_once: 'yes'

  # fails. should be "OK"
  - name: create timeperiod again
    checkmk.general.timeperiod:
      server_url: "{{ server_url }}"
      site: "{{ site }}"
      automation_user: "{{ automation_user }}"
      automation_secret: "{{ automation_secret }}"
      name: "lunch"
      alias: "Mahlzeit"
      active_time_ranges: [
              { 
                  "day": "all",
                  "time_ranges": [
                      {
                          "start": "12:00",
                          "end": "13:00"
                      }
                  ]
              }
          ]
      state: present
    delegate_to: localhost
    run_once: 'yes'

  - name: create timeperiod that uses the first one
    checkmk.general.timeperiod:
      server_url: "{{ server_url }}"
      site: "{{ site }}"
      automation_user: "{{ automation_user }}"
      automation_secret: "{{ automation_secret }}"
      name: "working_hours"
      alias: "Ballern"
      active_time_ranges: [
              { 
                  "day": "all",
                  "time_ranges": [
                      {
                          "start": "8:00",
                          "end": "17:00"
                      }
                  ]
              },
          ]
      exclude: ["Mahlzeit"]
      state: present
    delegate_to: localhost
    run_once: 'yes'

  - name: update timeperiod
    checkmk.general.timeperiod:
      server_url: "{{ server_url }}"
      site: "{{ site }}"
      automation_user: "{{ automation_user }}"
      automation_secret: "{{ automation_secret }}"
      name: "working_hours"
      active_time_ranges: [
              { 
                  "day": "all",
                  "time_ranges": [
                      {
                          "start": "9:00",
                          "end": "18:00"
                      }
                  ]
              },
          ]
      state: present
    delegate_to: localhost
    run_once: 'yes'

  # fails. should be "OK"
  - name: update timeperiod. again.
    checkmk.general.timeperiod:
      server_url: "{{ server_url }}"
      site: "{{ site }}"
      automation_user: "{{ automation_user }}"
      automation_secret: "{{ automation_secret }}"
      name: "working_hours"
      active_time_ranges: [
              { 
                  "day": "all",
                  "time_ranges": [
                      {
                          "start": "9:00",
                          "end": "18:00"
                      }
                  ]
              },
          ]
      state: present
    delegate_to: localhost
    run_once: 'yes'

  - name: remove used timeperiod
    checkmk.general.timeperiod:
      server_url: "{{ server_url }}"
      site: "{{ site }}"
      automation_user: "{{ automation_user }}"
      automation_secret: "{{ automation_secret }}"
      name: "lunch"
      state: absent
    delegate_to: localhost
    run_once: 'yes'
    ignore_errors: true

  - name: remove timeperiod
    checkmk.general.timeperiod:
      server_url: "{{ server_url }}"
      site: "{{ site }}"
      automation_user: "{{ automation_user }}"
      automation_secret: "{{ automation_secret }}"
      name: "working_hours"
      state: absent
    delegate_to: localhost
    run_once: 'yes'

  # fails. should be "OK"
  - name: remove timeperiod again
    checkmk.general.timeperiod:
      server_url: "{{ server_url }}"
      site: "{{ site }}"
      automation_user: "{{ automation_user }}"
      automation_secret: "{{ automation_secret }}"
      name: "working_hours"
      state: absent
    delegate_to: localhost
    run_once: 'yes'

  - name: remove used timeperiod
    checkmk.general.timeperiod:
      server_url: "{{ server_url }}"
      site: "{{ site }}"
      automation_user: "{{ automation_user }}"
      automation_secret: "{{ automation_secret }}"
      name: "lunch"
      state: absent
    delegate_to: localhost
    run_once: 'yes'

Max-checkmk · 2023-08-25T13:59:36Z

The documentation could be a bit more detailed. Probably, you could use the tasks from your integration test.

I tested the module with some use cases:
* When creating a timeperiod that already exists, a failure is shown (should be "OK").

* When updating a timeperiod without any actual change, the result is "changed" (should be "OK").

* When adding an exclude time range, this isn't visible in the UI.

* When deleting a timeperiod that doesn't exist, a failure is shown (should be "OK").

Hi Lars,

thanks for the feedback. I had a dumb mistake in it, which is now corrected.
And i forgot to finish the examples, which is also done now.

Please try it again.
What i didn't change is the error which is thrown, if someones tries to delete something what doesn't exist.
Not sure about this...

Nice weekend! ;)

msekania · 2023-08-25T14:42:42Z

What i didn't change is the error which is thrown, if someones tries to delete something what doesn't exist.
Not sure about this...

@Max-checkmk,

state: absent means that it's not there, that means that someone does not try to delete something but rather to ensure that it's not there, hence no error, rather no change. And changed if it was there and deleted.

lgetwan · 2023-08-25T15:04:58Z

plugins/modules/timeperiod.py

+            timeperiodupdate = TimeperiodUpdateAPI(module)
+            timeperiodupdate.headers["If-Match"] = result.etag


An Update should only be done in the case the mentioned parameters are different in the already existing Timeperiod.
Otherwise running the same playbook again and again will always do a change in the environment. You have to avoid this.

Nice idea, but this will need a lot of time to accomplish.

So as example we want to update a time period for all weekdays with these time ranges:
active_time_ranges: '[{"day": "all", "time_ranges": [{"start": "08:00:00", "end": "17:00:00"}]}]'

And we try to compare this with the existing time ranges of a time period, we get this:
[{'day': 'monday', 'time_ranges': [{'start': '08:00', 'end': '17:00'}]}, {'day': 'tuesday', 'time_ranges': [{'start': '08:00', 'end': '17:00'}]}, {'day': 'wednesday', 'time_ranges': [{'start': '08:00', 'end': '17:00'}]}, {'day': 'thursday', 'time_ranges': [{'start': '08:00', 'end': '17:00'}]}, {'day': 'friday', 'time_ranges': [{'start': '08:00', 'end': '17:00'}]}, {'day': 'saturday', 'time_ranges': [{'start': '08:00', 'end': '17:00'}]}, {'day': 'sunday', 'time_ranges': [{'start': '08:00', 'end': '17:00'}]}]

So we would need to adjust

the given time ranges to every single day of the week

remove the seconds from the input, as internally there are no seconds (in contrast to the API Docs example...)

We redirect the raw data to the API so at first you would have to convert the data to something you can work with in python.

I see, that's too complex.

But what do you think of this as an intermediate solution:
If the data provided by the task is completely equal with the current data from the API, we don't change anything. But if the task e.g. has seconds in the data, and the API has not, we do post a change to the API.

Many users are running their playbooks daily or hourly, and that would help them a lot, already.

Please have a look at my last commit. Now there is a rudimentary comparison between existing and new values.
Based on that we could add the other checks (full week and seconds) sometime.

lgetwan · 2023-08-25T15:05:40Z

Nice weekent y'all!

Max-checkmk · 2023-08-28T08:24:31Z

state: absent means that it's not there, that means that someone does not try to delete something but rather to ensure that it's not there, hence no error, rather no change. And changed if it was there and deleted.

You're right. I just had a look at some other modules and as far i see at the moment we're not really consistent for all modules here. ;)
I will do the change for this module with the next commit!

lgetwan

Works as described. :-)

robin-checkmk · 2023-10-16T09:38:12Z

@Max-checkmk please check the failing sanity check and fix it. I think we had this issue with the host or folder module in the past. Maybe @lgetwan can help here.

Max-checkmk · 2023-10-16T09:43:50Z

@Max-checkmk please check the failing sanity check and fix it. I think we had this issue with the host or folder module in the past. Maybe @lgetwan can help here.

Well, the problem seems to be inside the folder-module and not in this module. As far as i can read the logs from the test...

robin-checkmk · 2023-10-16T10:25:08Z

True. Then we will ignore the failing test here and fix it in devel directly.

New timeperiod module

cbebc86

Max-checkmk added enhancement New feature or request release:3.3.0 Affects the mentioned release. module:timeperiod This affects the timeperiod module labels Aug 25, 2023

Max-checkmk self-assigned this Aug 25, 2023

Max-checkmk requested review from lgetwan and robin-checkmk as code owners August 25, 2023 09:24

Corrected imports

4cabde4

lgetwan requested changes Aug 25, 2023

View reviewed changes

Corrected some mistakes and added infos

5e33e8f

lgetwan requested changes Aug 25, 2023

View reviewed changes

Max-checkmk and others added 2 commits August 28, 2023 10:54

Delete with non existing timeperiod doesn't fail.

553a005

Added comparison before update

d93dfec

lgetwan approved these changes Aug 31, 2023

View reviewed changes

robin-checkmk mentioned this pull request Aug 31, 2023

Release 3.3.0 #434

Merged

7 tasks

Max-checkmk added 2 commits October 5, 2023 15:37

finished compare function for idempotence

a4c4f2c

black correction

77d9403

Max-checkmk requested a review from lgetwan October 5, 2023 13:47

Max-checkmk mentioned this pull request Oct 6, 2023

Version comparison #443

Merged

7 tasks

robin-checkmk merged commit 52c8747 into devel Oct 16, 2023
7 of 8 checks passed

github-actions bot locked and limited conversation to collaborators Oct 16, 2023

robin-checkmk deleted the feature/timeperiod_module branch October 16, 2023 10:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New time period module #426

New time period module #426

Max-checkmk commented Aug 25, 2023

lgetwan left a comment

Max-checkmk commented Aug 25, 2023

msekania commented Aug 25, 2023 •

edited

Loading

lgetwan Aug 25, 2023

Max-checkmk Aug 28, 2023

lgetwan Aug 29, 2023

Max-checkmk Aug 30, 2023

lgetwan commented Aug 25, 2023

Max-checkmk commented Aug 28, 2023

lgetwan left a comment

robin-checkmk commented Oct 16, 2023

Max-checkmk commented Oct 16, 2023

robin-checkmk commented Oct 16, 2023

		timeperiodupdate = TimeperiodUpdateAPI(module)
		timeperiodupdate.headers["If-Match"] = result.etag

New time period module #426

New time period module #426

Conversation

Max-checkmk commented Aug 25, 2023

Pull request type

What is the current behavior?

What is the new behavior?

Other information

lgetwan left a comment

Choose a reason for hiding this comment

Max-checkmk commented Aug 25, 2023

msekania commented Aug 25, 2023 • edited Loading

lgetwan Aug 25, 2023

Choose a reason for hiding this comment

Max-checkmk Aug 28, 2023

Choose a reason for hiding this comment

lgetwan Aug 29, 2023

Choose a reason for hiding this comment

Max-checkmk Aug 30, 2023

Choose a reason for hiding this comment

lgetwan commented Aug 25, 2023

Max-checkmk commented Aug 28, 2023

lgetwan left a comment

Choose a reason for hiding this comment

robin-checkmk commented Oct 16, 2023

Max-checkmk commented Oct 16, 2023

robin-checkmk commented Oct 16, 2023

msekania commented Aug 25, 2023 •

edited

Loading