Skip to content
This repository has been archived by the owner on Jun 10, 2024. It is now read-only.

Public Ansible Project Meeting Agenda - January 2018 #291

Closed
thaumos opened this issue Dec 19, 2017 · 29 comments
Closed

Public Ansible Project Meeting Agenda - January 2018 #291

thaumos opened this issue Dec 19, 2017 · 29 comments

Comments

@thaumos
Copy link

thaumos commented Dec 19, 2017

Please leave a comment regarding any agenda item you wish to discuss. If you don't show up for the meeting, your item will be skipped.

If your IRC nick is different from your Github username, leave that as well.

See https://github.com/ansible/community/blob/master/meetings/README.md for the schedule

Once an item has been addressed it should get strike-though ~~strike-though~~

When creating new agenda ensure core and meeting_agenda labels are set

@itdependsnetworks
Copy link

itdependsnetworks commented Jan 3, 2018

We will revisit this for the 2.7 release.

Want to start back the conversations on gather_facts, current issue opened: ansible/ansible#34109

Some backstory:

Original
ansible/ansible#20717

Meeting where it was previously decided not to change.
https://meetbot.fedoraproject.org/ansible-meeting/2017-07-13/core_meeting.2017-07-13-15.00.log.html

Relevant IRC logs:

[2017-04-20T12:45:26-0400] <sivel> I think regardless of 20717, there will still be a long tail on `setup`, since many people call setup directly
[2017-04-20T12:45:33-0400] <sivel> it may fix some problems immediately, but not all
[2017-04-20T12:45:58-0400] <@bcoca> sivel: _setup will be found for those using the action in module, but won't trip setuptools
[2017-04-20T12:46:08-0400] <@bcoca> ^ aliases!
[2017-04-20T12:46:13-0400] <@bcoca> first thing i implemented when hired
[2017-04-20T12:46:27-0400] <sivel> yeah, I was saying, if you still had a `setup.py`, people calling `setup` directly could still have problems
[2017-04-20T12:46:53-0400] <@bcoca> yes as it would still override
[2017-04-20T12:47:25-0400] <@bcoca> hence the 'long tail' of really getting rid of setup as we would have to push people to stop using it
[2017-04-20T12:48:10-0400] <@bcoca> itdependsnetwork: and at most it will be 2.4, we dont add features like that in minor versions
[2017-04-20T12:48:12-0400] <itdependsnetwork> ok, so that is why it is so long. It's not just "gather_facts: yes"
[2017-04-20T12:48:20-0400] <@abadger1999> bcoca: It will trip setuptools I think.  But the effect will be minimized
[2017-04-20T12:48:25-0400] <@bcoca> itdependsnetwork: if it were just that .. trivial
[2017-04-20T12:48:38-0400] <itdependsnetwork> ok, did not understand that
[2017-04-20T12:48:40-0400] <@bcoca> abadger1999: yes, that is the purpose of the PR, it will only trip for people explicitly using setup
[2017-04-20T12:48:42-0400] <@abadger1999> I think _setup.py and setup.py will still conflict in PluginLoader
[2017-04-20T12:48:58-0400] <@abadger1999> btut since ansible internally is calling gather_facts, it won't run into it internally
[2017-04-20T12:49:02-0400] <@abadger1999> <nod>
[2017-04-20T12:49:03-0400] <@abadger1999> yep.
[2017-04-20T12:49:05-0400] <@bcoca> my Pr makes setup an alias to allow for explicit calls, but removes it from implicit
[2017-04-20T12:49:05-0400] <@abadger1999> agreed
[2017-04-20T12:49:48-0400] <@bcoca> so it shoudl solve the problem for those in conflict with minor adjustment to playbooks that call setup explicitly
[2017-04-20T12:49:57-0400] <itdependsnetwork> If I am only worried about people doing a "gather_facts: yes" does 20717 fix?
[2017-04-20T12:50:00-0400] <@bcoca> for rest ... we need deprecation notice
[2017-04-20T12:50:08-0400] <@bcoca> itdependsnetwork: yes
[2017-04-20T12:50:13-0400] <itdependsnetwork> +1
[2017-04-20T12:50:23-0400] <itdependsnetwork> that is all I am worried about
[2017-04-20T12:50:40-0400] <@bcoca> sadly, we have to worry about more cases
[2017-04-20T12:50:42-0400] <@bcoca> but its a start
[2017-04-20T12:50:46-0400] <itdependsnetwork> I know :)
[2017-04-20T12:51:16-0400] <itdependsnetwork> and it is still considered too large to be considered for 2.3.X?

As per 6 month old subject to change info from: @thaumos should be looking to change in 2.5

[2017-07-13T12:16:54-0400] <itdependsnetwork> just realized meeting is going on, did I miss facts vs setup talk?
[2017-07-13T12:17:02-0400] <thaumos> yep, issue was closed.
[2017-07-13T12:17:12-0400] <itdependsnetwork> meaning resolved?
[2017-07-13T12:17:20-0400] <thaumos> meaning we aren't merging in the change.
[2017-07-13T12:17:25-0400] <itdependsnetwork> why?
[2017-07-13T12:17:39-0400] <thaumos> because the plan is to change the way facts work in the near future.
[2017-07-13T12:17:54-0400] <thaumos> so it seemed silly to bikeshed around a name to have for something that won't exist in the future.
[2017-07-13T12:18:01-0400] <itdependsnetwork> in the meantime I am dealing with user issues that no one want to fix
[2017-07-13T12:18:12-0400] <itdependsnetwork> not so silly to me
[2017-07-13T12:18:43-0400] <thaumos> the intent is to change to the new framework within a release or two.
[2017-07-13T12:19:04-0400] <itdependsnetwork> I get it, I won't win
[2017-07-13T12:19:18-0400] <itdependsnetwork> just real frustrating on what will or won't get fixed
[2017-07-13T12:20:20-0400] <thaumos> It will get fixed by changing the framework.  The concept of "setup" as a module keyword won't exist in the next release.
[2017-07-13T12:21:28-0400] <itdependsnetwork> so there will or will not be a setup.py file in ansible module path in 2.4?
[2017-07-13T12:21:38-0400] <thaumos> in 2.4, there will be.
[2017-07-13T12:22:01-0400] <thaumos> in 2.5 there will most likely not

EDIT: @itdependsnetworks, thank you for bringing this to our attention again. This has been decided to put on hold until 2.7. I will cover this again on the 19 April meeting to start discussions on how to implement for that release.

@jvanderhoof
Copy link

jvanderhoof commented Jan 3, 2018

I'd like to talk through next steps for PR: ansible/ansible#34280

@abadger
Copy link
Contributor

abadger commented Jan 3, 2018

Would like a discussion and vote to deprecate and remove DEFAULT_MODULE_LANG and DEFAULT_MODULE_SET_LOCALE. MODULE_LANG was implemented to allow modules to screenscrape from CLI utilities that they invoked. However, that never worked well because our defaults used the local machine's locale which made the remote messages vary from user to user. We then created helper functions so that modules could set those environment variables prior to invoking any commands via AnsibleModule.run_command() and ported modules to use that. This was so successful that when people complained that we were setting LC_ALL, we were able to create MODULE_SET_LOCALE to let them completely disable the use of MODULE_LANG and modules continued to work. In 2.2 we changed the default of MODULE_SET_LOCALE to False and no one seems to have noticed. I think it's time that we deprecate these settings and get rid of them in 4 releases.

Deprecating this was approved at the January 9th meeting.

@pilou-
Copy link
Contributor

pilou- commented Jan 4, 2018

I would like to discuss:
- ansible/ansible#25519 (filesystem module)
- ansible/ansible#33967 (add and use check_file_absent_if_check_mode method in lib/ansible/module_utils/basic.py)

discussed (1) and (2) was already merged.

@maxamillion
Copy link
Contributor

maxamillion commented Jan 4, 2018

ansible/ansible#33177

In Ansible 2.3, it appears that the function _get_hostgroup_vars from the Inventory class handled the scenario of a playbook that included another playbook and kept the path of the original playbook in scope. However, in 2.4 with the transition to inventory plugins, the basedirs for the playbook are determined by the DataLoader which doesn't appear to have any knowledge of the original playbook that did the include/import. I would like to discuss a preferred method to handle playbook adjacent group_vars directories in Ansible >2.4.

(Thanks to bcoca, jimi-c, and sivel for helping me get to the root cause of this)

Decision: we won't bring this back, it's been classified as a bug that has been resolved. Porting guide for 2.4 will be updated.

@sivel
Copy link
Member

sivel commented Jan 4, 2018

ansible/ansible#34427

LooseVersion comparison in py3 is subject to exceptions due to comparing int and str. That PR implements a potential fix, but it applies elsewhere too, not just to galaxy role version comparison.

Do we go with that PR, move the new class to a different location for use elsewhere (module_utils)? Or do we want to think about using pypa/packaging instead. SafeLooseVersion would be easier to ship in module_utils for modules that can utilize it.

Going to handle these LooseVersion issues on a case by case basis for now. Will evaluate galaxy version spec plans

@gundalow
Copy link
Contributor

gundalow commented Jan 8, 2018

Deprecation

While working on ansible/ansible#34100 it seems there is some confusion around what version means in terms of deprecation

Currently we have a few ways of notifying of deprecation in Ansible:

  • Module deprecation (git mv $foo.py _$foo.py & deprecation: in DOCUMENTATION block)
  • module.deprecate(("State 'removed' is deprecated. Using state 'absent' instead.", version="2.9")
  • display.deprecated('Use foo instead', version='2.4')
    • version here is used inconsistently to mean start or end of cycle.
  • Deprecation in config
  • anywhere else?

Since the original code was added for the above we have versioned documentation. Which makes some of the easier.

There is confusion if version means "start of deprecation cycle" or "end of deprecation cycle"

Questions/ideas

  1. Should the above all mean the same

    1. will_be_deleted_in_version
      • = (version of devel + 4)
      • If we ignore that there may not be a 2.9, and we jump straight to 3.0, is this OK?
    2. deprecated_as_of_version
      • When the code was updated = deprecated as of version
      • We now have versioned documentation
      • Are there ever any cases of wanting to give notice that something will be deleted, but we don't know exactly which release?
  2. Should we rename the variable to be clearer

  3. Better docs

    1. Deprecated modules to link to the porting page
    2. Porting page to link to Maintenance and releases page
  4. When devel version++ how can we detect the three cases and git rm as needed?

Porting page: http://docs.ansible.com/ansible/devel/porting_guides.html

Maintenance and releases page: https://docs.ansible.com/ansible/devel/release_and_maintenance.html

At today's meeting we decided to:

  • Rename the version parameter in all deprecation APIs to removed_in
  • Keep an aliased name of "version" until we've gone through all of the calls to make sure they're doing it right.
  • Over time we'll check that deprecation calls are using the right version information (version to remove in, not version it was deprecated in) and change "version" to "removed_in" so we know that someone has looked at that.
  • Remove the "version" parameter alias once that's finished.
  • gundalow will update the versions in documentation as that's all been vetted already
  • gundalow will document the proper use of deprecations

@andreaso
Copy link

andreaso commented Jan 8, 2018

Would like to bring up the cloudflare_dns bugfix ansible/ansible#31800, and woulat it would take to get it merged.

My IRC nick is andol.

@gundalow
Copy link
Contributor

gundalow commented Jan 8, 2018

The 2.5 freeze dates (merge deadlines) have been pushed out by a week.

Please see updated dates
https://github.com/ansible/ansible/blob/devel/docs/docsite/rst/roadmap/ROADMAP_2_5.rst

To see whats change please see ansible/ansible@1826c27

@tdtrask
Copy link

tdtrask commented Jan 11, 2018

I would like to discuss ansible/ansible#34325 and the fix at ansible/ansible#34419. The fix ensures that any packages set to 'present' are installed as top-level packages, not just as dependencies for other packages. The new concern raised by @abadger is for state 'absent'. For the Alpine Linux apk package manager, it is not possible to uninstall a package that is installed as a dependency. In fact, the add and del subcommands do not directly install or remove packages. What they do is add rules to the /etc/apk/world file, which are then implemented by the package manager based upon dependencies. The question is whether 'absent' should fail if the package is still installed as a dependency of another package.

Update of what happened in last week's meeting:

  • Asked that tdrask find an actual user of the module so that we can get some feedback from them about what they expect and then we can evaluate which way this should work.

@gundalow
Copy link
Contributor

gundalow commented Jan 11, 2018

Proposal: Standard way to influence return values
From @dagwieers ansible/proposals#93

At today's meeting we decided that we don't currently have enough use cases to tell if there should be an ansible-wide standard for this. Talked with dag about various ways that might work for the ACI modules as a local standard.

@dagwieers
Copy link
Contributor

dagwieers commented Jan 11, 2018

Another one: ansible/ansible#26889

We talked about this at today's IRC meeting and it seemed most people were okay with addressing this use case. However, we were resistant to adding a new module/action plugin for it. If this could be merged into the existing pause or wait_for modules/action_plugins, that seems like something we can look at..

@sivel
Copy link
Member

sivel commented Jan 18, 2018

Inventory paths with commas: ansible/ansible#35002

@Akasurde
Copy link
Member

Akasurde commented Jan 18, 2018

I would like to discuss an idea about new label to identify work progress of issue. This label can be applied to issue which has an open PR. Something like has_pr or working.

This will identify if there is a PR or not without even opening this issue's details page.

We've created has_pr and will move forward with doing it manually during triage, and get bot functionality added.

@gundalow
Copy link
Contributor

gundalow commented Jan 18, 2018

Autogenerated modules

ansible/ansible#35014 (gundalow, 15:06:51)

* #35014 has till the "community" freeze as a) None of GCP is support: core b) module_util changes are only to new in 2.5 modules, therefore no risk of breaking anything else (gundalow, 15:47:10)

* OPTION 1) We have the bot and ANSIBLE_METADATA dictate auto closing issues or PRs for these modules. (requires a proposal)
* OPTION 2) 100% ignoring that they are auto generated (no exceptions), no pointing people elsewhere, just forgetting where they came from, treated like any other module
Remember to consider how this applies to existing auto generated modules, such as network/avi

@maxamillion
Copy link
Contributor

maxamillion commented Jan 24, 2018

An user suggests that the service module should not error when taking action on services that are not installed on the remote host, but I disagree. However, I would be interested in the consensus from the community on this topic.

ansible/ansible#35293

Decision is to leave behavior as is (failing when the service does not exist)

@thaumos
Copy link
Author

thaumos commented Jan 24, 2018

Vote on changing Thursday's time from 1500 UTC. Suggesting 1600 or 1700 UTC.

  • Dag brought up the good point that making it later moves the times out of business hours for parts of Europe. 16:00 is still out of business hours for the US West Coast, too. Comments were made about the Thursday meeting getting cancelled a lot due to not getting enough attendance. There were some thoughts about changing the purpose of the meeting but all of those have some component of needing to have the right people there to discuss and refine proposals....

  • For now, we took a vote on moving the thursday meeting time to 16:00 UTC, +1:6, 0:1, -1:1. Will also collect more votes amongst those that attend on Thursday to see if the Tuesday vote is skewed because those are the people who have a hard time making it thursday. If we decide to move the time, that would happen next week.

  • Can discuss other ideas about changing meetings/discussions/etc after the vote on the meetings is done.

EDIT Feb 1st: 2 more votes collected, bringing total to +1:6 , 0:2 , -1:2

@maxamillion
Copy link
Contributor

+1 to either 1600 or 1700 UTC

@eikef
Copy link

eikef commented Jan 25, 2018

I'd like to request a further review of ansible/ansible#33419 and merge if possible.

  • sivel is doing a code review.
  • eikef is going to get a user to test and see if it the API makes sense to them

@Akasurde
Copy link
Member

Akasurde commented Jan 29, 2018

I would like to discuss these issues - related to Python 3.6 and LooseVersion. Following issues are occurred due to this -

Basic cause of this is Python Bug

  • The latter two are galaxy issues that we've already made a decision on how to address.
  • The decision is to continue handling these on a case by case basis. For hostname we'll drop LooseVersion in favor of int casting, and fall back to UnimplementedStrategy

@evrardjp
Copy link

evrardjp commented Jan 29, 2018

I'd like to discuss the inclusion of config_template as a community action plugin.
ansible/ansible#35453 .

Discussion from previous PR ansible/ansible#12555 (comment)

The decision here was that we are not sold on the current action plugin. Instead the recommendation would be to add yaml and json modules (not action plugins) that operate in the same manner as ini_file and xml.

@sivel
Copy link
Member

sivel commented Jan 30, 2018

Should we support old TLS/SSL versions: ansible/ansible#35462

  • We made progress:
    • Decided that we do need to support old TLS/SSL somehow
    • Decided that the defaults should be secure but people can enable the insecure behaviour for old devices.
    • Tentatively agreed that changing module args is best because it allows for the best granularity (programmer can choose being able to choose which calls to fetch_url() allow insecure behaviour. User can choose which tasks need to use insecure behaviour).
    • Unsure about how to make wider defaults. There are risks involved with that (Downgrade attacks mean that we should not enable this globally even if the user asks. Global would affect any uses of get_url, even for tasks that are otherwise okay to use with new protocols).
    • Timeframe is 2.7 or possibly 2.6. needs more features exposed in module_utils/urls.py and needs support in individual modules to give users a parameter to adjust and then pass that on to fetch_url/open_url inside of urls.py.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests