{"action":"created","issue":{"url":"https://api.github.com/repos/ansible/proposals/issues/70","repository_url":"https://api.github.com/repos/ansible/proposals","labels_url":"https://api.github.com/repos/ansible/proposals/issues/70/labels{/name}","comments_url":"https://api.github.com/repos/ansible/proposals/issues/70/comments","events_url":"https://api.github.com/repos/ansible/proposals/issues/70/events","html_url":"https://github.com/ansible/proposals/issues/70","id":258004882,"number":70,"title":"Hold off and redesign Declarative Intent","user":{"login":"dagwieers","id":388198,"avatar_url":"https://avatars0.githubusercontent.com/u/388198?v=4","gravatar_id":"","url":"https://api.github.com/users/dagwieers","html_url":"https://github.com/dagwieers","followers_url":"https://api.github.com/users/dagwieers/followers","following_url":"https://api.github.com/users/dagwieers/following{/other_user}","gists_url":"https://api.github.com/users/dagwieers/gists{/gist_id}","starred_url":"https://api.github.com/users/dagwieers/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dagwieers/subscriptions","organizations_url":"https://api.github.com/users/dagwieers/orgs","repos_url":"https://api.github.com/users/dagwieers/repos","events_url":"https://api.github.com/users/dagwieers/events{/privacy}","received_events_url":"https://api.github.com/users/dagwieers/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":23,"created_at":"2017-09-15T11:08:02Z","updated_at":"2017-09-20T09:43:15Z","closed_at":null,"author_association":"MEMBER","body":"<!--- Follow this template to create a proposal. -->\r\n\r\n# Proposal: Redesign Declarative Intent\r\n\r\n*Author*: Dag Wieers <@dagwieers> IRC: dag\r\n\r\n*Date*: 2017/09/15\r\n\r\n- Status: New\r\n- Proposal type: core design\r\n- Targeted Release: 2.5\r\n- Associated PR: https://github.com/ansible/ansible/pull/26954/files\r\n- Estimated time to implement: ?\r\n\r\n## Motivation\r\nDuring AnsibleFest SF 2017 it was clear that the Declarative Intent functionality that was designed as part of needed ansible-network roadmap is flawed in a few areas.\r\n\r\nThis proposal does not question the merit of this needed functionality, but some design decisions require improvements before this is ready for wider consumption.\r\n\r\n- The implementation diffuses the configuration namespaces with the expectation declaration, which is going to be very confusing. Not just because some of these parameters are used elsewhere for configuration, but simply because everything is a top-level module parameter. This is bad user interface design. The remark that the documentation will make clear what parameters are part of the configuration declaration and what parameters are expectations does not make this acceptable.\r\n\r\n  The example from the documentation:\r\n\r\n  ```yaml\r\n  - name: configure interface\r\n    net_interface:\r\n      # Declared configuration\r\n      name: GigabitEthernet0/2\r\n      description: public interface configuration\r\n      enabled: yes\r\n\r\n      # Intended state\r\n      state: connected\r\n      txrate: 10gb\r\n    ```\r\n\r\n    So **state** here is an expected state of the interface, and **tx_rate** is the expected transmit rate, not and actual declarative configuration or even an intention as such. A better interface would be:\r\n\r\n  ```yaml\r\n  - name: configure interface\r\n    net_interface:\r\n      # Declared configuration\r\n      name: GigabitEthernet0/2\r\n      description: public interface configuration\r\n      enabled: yes\r\n\r\n      # Expected state\r\n      expect:\r\n        link: connected\r\n        tx_rate: 10gb\r\n    ```\r\n\r\n- Declarative intent seems to indicate we are declaring our intentions. But that's not what it does, it declares expectations rather than intentions. If we look at the above examples the items listed are not (as documented) the \"Intended state\", but rather the \"Expected state\" an interface is in.\r\n\r\n  \"Intended state\" seems to imply it is something we can control, where \"expected state\" better describes what it is. An intention is something one contributes to, an expectation is something we consider getting back. So the functionality a misnomer in itself.\r\n\r\n  From the documentation:\r\n\r\n  > Declarative intent modules also take a set of options that allow playbook designers to specify the desired or intended operational state of the resource.  These options allow tasks to define the normalized operating state of the resource once the configuration has been completed.  **State options do not make configuration changes** to the resource.  They serve to validate that a given resource has normalized its operating environment to reflect the intended state.  If the operating state does not reflect the intended (or desired) state of the resource as described in the playbook task, the module is considered failed and remediation actions can be taken in the playbook during the playbook execution.\r\n\r\n- There is another problem with doing both configuration and expectation checking in a single module. If one of both operations fail, how would we know if the configuration failed, or the expectations were not met. And should a failed expectation be a module failure or not ? I can see different use-cases for testing expectations and as such, it would be more versatile of expectations would be handle by special tasks rather than being part of the existing modules.\r\n\r\n  Or maybe the core Ansible product should understand the difference between config and expectation and could be told to either ignore/warn on expectation failures, or have a different failure path. Rather than diffuse the handling of the module intrinsically. Maybe you don't want to run expectation tests in some runs (e.g. production updates), but you want them in other runs (like integration tests, or compliance testing).\r\n\r\n- This functionality is useful in a wider context than just networking, so this should have been discussed with use-cases outside of networking only. And this maps with the existing **assert** module and registering the return values. If we agree that doing a validation check as part of the call, we could leave this up to Ansible core to do the test (and take the appropriate action):\r\n\r\n  ```yaml\r\n  - name: configure interface\r\n    net_interface:\r\n      name: GigabitEthernet0/2\r\n      description: public interface configuration\r\n      enabled: yes\r\n    expect:\r\n      link: connected\r\n      tx_rate: 10gb\r\n  ```\r\n\r\n  This then would make writing integration tests easier, and ensures that other modules would automatically have this functionality without requiring any special modifications. This is a much stronger use-case than the original implementation.\r\n\r\n### Problems\r\nWhat problems exist that this proposal will solve?\r\n- Releasing and documenting this as-is (with the known problems), will make a future implementation harder to get right and requires backward compatibility.\r\n\r\n## Solution proposal\r\n- Redesign this functionality with other use-cases in mind\r\n- Redesign this with all stakeholders involved\r\n- Redesign this with regard to good user interface design\r\n\r\n## Documentation (optional)\r\nHold off with documenting and promoting this as part of v2.4, so we don't have to support this going forward in light of a user-breaking redesign.\r\n\r\n## Anything else?\r\nThe above feedback was provided during the AnsibleFest SF 2017 Network Working Group and was largely unanimous by participants.\r\n\r\nThis proposal was made to collect feedback from other participants in order to design a better user interface for this functionality before it becomes a wider standard."},"comment":{"url":"https://api.github.com/repos/ansible/proposals/issues/comments/330801396","html_url":"https://github.com/ansible/proposals/issues/70#issuecomment-330801396","issue_url":"https://api.github.com/repos/ansible/proposals/issues/70","id":330801396,"user":{"login":"dagwieers","id":388198,"avatar_url":"https://avatars0.githubusercontent.com/u/388198?v=4","gravatar_id":"","url":"https://api.github.com/users/dagwieers","html_url":"https://github.com/dagwieers","followers_url":"https://api.github.com/users/dagwieers/followers","following_url":"https://api.github.com/users/dagwieers/following{/other_user}","gists_url":"https://api.github.com/users/dagwieers/gists{/gist_id}","starred_url":"https://api.github.com/users/dagwieers/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dagwieers/subscriptions","organizations_url":"https://api.github.com/users/dagwieers/orgs","repos_url":"https://api.github.com/users/dagwieers/repos","events_url":"https://api.github.com/users/dagwieers/events{/privacy}","received_events_url":"https://api.github.com/users/dagwieers/received_events","type":"User","site_admin":false},"created_at":"2017-09-20T09:43:15Z","updated_at":"2017-09-20T09:43:15Z","author_association":"MEMBER","body":"@bcoca I agree with most of what you say, my only objections (and for me trumps mostly everything else) is convenience and complexity.\r\n\r\nWriting a role per task for this seems inconvenient, even if these roles already exist (for every network device ?) there is a burden (selecting, trusting, managing roles) and the complexity understanding and troubleshooting issues when they arise. If you abstract away through roles, and it fails, the error throws you in that complexity.\r\n\r\nAnd the great strength of Ansible is that it makes hard/complex tasks easy through a simple interface. Do we move the complexity into roles (user-facing) or in the core (abstracted away in a single task) ?\r\n\r\nIf my example was too limited compared to **assert** that's solved by doing this:\r\n```yaml\r\n- name: configure interface\r\n  net_interface:\r\n    name: GigabitEthernet0/2\r\n    description: public interface configuration\r\n    enabled: yes\r\n  assertions:\r\n  - this.link == 'connected'\r\n  - this.tx_rate == '10gb'\r\n```\r\n\r\nTrue, it doesn't offer the same as one or more tasks could offer, but maybe it fits 80% of the use-cases."}}
{"actor":{"display_login":"dagwieers"}}