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

(design_proposal): chef cli catalog #877

Merged
merged 15 commits into from
Feb 21, 2020
Merged

Conversation

afiune
Copy link

@afiune afiune commented Jan 23, 2020

Description

A design proposal for the future of the chef CLI Catalog.

Motivation

As a Chef Operator,
I need a unified Command Line Interface (CLI) that lets me interact with every Chef product,
so I can easly discover the capabilities of the Chef ecosystem.

As a Chef Developer,
I need to be able to gather information about how our users consume and interact with our tools,
so I can identify which commands are used the most and measure the usability and performance on all supported platforms.

Acceptance Criteria

  • A design doc is stored inside the code repo
  • The design doc has been approved by everyone on the team
  • Design doc has enough details / shared understanding that any engineer could write correctly scoped cards based on it
  • Update the linked epic card to capture the scope of this design

Signed-off-by: Salim Afiune afiune@chef.io

@afiune afiune requested a review from a team as a code owner January 23, 2020 20:12
@afiune afiune force-pushed the design_proposal/chef_cli_catalog branch from 5e335b3 to 7d6dfc8 Compare January 23, 2020 20:14
@afiune afiune requested a review from a team January 23, 2020 20:20
@afiune afiune self-assigned this Jan 23, 2020
@afiune afiune added Triage: Confirmed Indicates and issue has been confirmed as described. Type: Design Proposal Community survey of a proposal. labels Jan 23, 2020
Copy link
Contributor

@jonsmorrow jonsmorrow left a comment

Choose a reason for hiding this comment

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

Overall I like the general idea and shape here. Anytime we do design proposals we want to be able to defend questions like:
What other solutions to this problem where considered?
Are there simpler options or steps along the way that provide value and help us evaluate the path?

I think we can work on the problem statement and it will help with these.

What jumps out to me is a more measured approach where we get telemetry into each of these commands. At least enough to know when they are called. This might influence how/if we brig something under the top level chef command. We could do this with go wrappers and a go telemetry library or maybe look at the ruby bins already generated and see if we can insert the existing library.

# Chef CLI (Catalog)

Throughout the history of Chef, we have created and acquired a variety
of tools that have grown to a point that, as a Chef practitioner, it is
Copy link
Contributor

Choose a reason for hiding this comment

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

Chef Operator to bring this in-line with our current target user base.


## Goals
* Have a single, unified way to discover tools to interact with Chef products
* Huge speed improvements
Copy link
Contributor

Choose a reason for hiding this comment

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

All of the other goals are more tangible. I'd say something like: "Consistent/Usable performance on all supported platforms"

I would have a tremendous benefit from this architecture since
I would be able to gather information about how our users consume and interact with our tools,
as well as identifying the commands that are used the most,
so I can improve the speed and/or do extreme modifications to the code base (do a rewrite in a different, more efficient, programing language)
Copy link
Contributor

Choose a reason for hiding this comment

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

This so states things you could do not why you'd want to. Is there a way to say this without talking about extreme changes. Anytime we use that kind of language it can alienate folks that want more measured changed/no change. It's ok if you really mean to drive extreme change and understand the above, but make sure that's the intent.

Finally, since I run these tools daily, I need them to run fast

As a Chef developer,
I would have a tremendous benefit from this architecture since
Copy link
Contributor

Choose a reason for hiding this comment

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

These I woulds are future tense assuming this has been implemented. I think they need to be written as what is desired. It's also pre-judging the architecture before garnering feedback. I'd suggest more along the lines of: I would like a more flexible architecture that allows me to invest in improvements selectively where they will have the most impact.


## Specification

Currently, inside the latest version of Chef Workstation (0.14.16 as of 01/20/2020),
Copy link
Contributor

Choose a reason for hiding this comment

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

It's been there longer, so I'd find when it landed or just remove the version.

Copy link
Author

Choose a reason for hiding this comment

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

yup, since 0.10.41


Currently, inside the latest version of Chef Workstation (0.14.16 as of 01/20/2020),
the team has implemented a top-level chef command that acts as a wrapper around our
variety of tools. The main idea about this command is to have the ability to create
Copy link
Contributor

Choose a reason for hiding this comment

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

wrapper around chef commands.

Choose a reason for hiding this comment

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

is it correct to assume the wrapper has been applied to all commands for all binaries included in workstation?


Currently, inside the latest version of Chef Workstation (0.14.16 as of 01/20/2020),
the team has implemented a top-level chef command that acts as a wrapper around our
variety of tools. The main idea about this command is to have the ability to create
Copy link
Contributor

Choose a reason for hiding this comment

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

Here you are future tense again as if it's been done. I think you want to say something like this design proposes we expand this scope to be a proxy/catalog of all the user facing cli tools we ship.


![chef-cli-catalog](img/chef-top-level-command.jpg)

With this architecture, we are now able to integrate and modularize multiple tools
Copy link
Contributor

Choose a reason for hiding this comment

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

s/we are now able/we would be able

Once detecting such areas of improvement, we can have the flexibility to improve
tools entirely or sub-sections of the tool. Say for instance, that our users run
`knife search` around 30-50 times a day, if the command takes around 10-15
seconds to run, we are consuming ~10 minutes from our users only on waiting time.
Copy link
Contributor

Choose a reason for hiding this comment

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

~10 minutes a day from our users waiting for value.

Choose a reason for hiding this comment

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

Can someone help me understand how catalog improvements are links to performance? (i.e. how does the catalog work speeds things up)

### Implement Telemetry into the top-level chef command
We need to start gathering information about how our users use the chef CLI,
this will allow us to understand the impact that we will have from the
aggressivemoves we are about to do.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we do this in a non-aggressive way?

Choose a reason for hiding this comment

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

With regard to telemetry, how can we link this to legal requirements around data capture so the design doc includes the our internal privacy requirements? (e.g. how can we log requirements from legal such as only turn on anonymous data capture after accepting the EULA?)

TBA

## Milestones
### Implement Telemetry into the top-level chef command

Choose a reason for hiding this comment

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

Just FYI, there's some dependency here on business requirements and legal requirements. Essentially we need to solidify our business requirements, get buy in from potential stakeholders (i.e. outside of Prod/Eng) and then get legal approval to proceed. This work is essential but just want to call out that the requirements are a little TBD right now and we have legal hurdles to overcome.

Throughout the history of Chef, we have created and acquired a variety
of tools that have grown to a point that, as a Chef practitioner, it is
hard to know, discover and understand all of them. A few of the tools
that our users use (almost) every day are:

Choose a reason for hiding this comment

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

I think having a catalog of all available commands/tools is very important. Once the user becomes aware of a tool, the next step is really for them to figure out what it does and if it's useful to them. Is that part of the intended scope of this work? If so then we can collaborate w/ UX team on figuring where/how to display relevant information about each tool and maybe include URLs to more detailed information (e.g. docs page) to users

Copy link
Author

Choose a reason for hiding this comment

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

Yes, this is exactly one of the ideas/goals and therefore, collaboration with UX will be required.

@jonsmorrow
Copy link
Contributor

Jon will take point on driving this with Jessica and coordinating with salim for any updates to the design.

@afiune afiune added the Status: Adopted An issue that is being worked on. label Jan 27, 2020
@afiune afiune force-pushed the design_proposal/chef_cli_catalog branch from 7aca044 to 6ea09ea Compare February 18, 2020 17:09
@afiune afiune force-pushed the design_proposal/chef_cli_catalog branch 2 times, most recently from 7479044 to 08e0dcc Compare February 18, 2020 19:24

## Open points for further discussion:
* Are we discouraging the use of individual sub-binaries like `knife` or `chef-run`?
* What happens when a user runs a sub-binary without the prefix `chef`?
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO, we should not communicate the existence of these binaries to users but we should not prevent their usage. It is just unsupported usage.

Copy link
Contributor

Choose a reason for hiding this comment

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

I misunderstood this line - I was thinking that we should document chef policy and not document chef-policy but if they run chef-policy we don't do anything to prevent them from doing that.

install Use now: chef policy install
update Use now: chef policy update
push Use now: chef policy push
push-archive Use now: chef policy push-archive
Copy link
Contributor

Choose a reason for hiding this comment

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

Going with the CLI UX noun verb guidelines I think the following make more sense:

chef policy install
chef policy update
chef policy push
chef policy show
chef policy diff
chef policy export
chef policy delete
chef policy undelete
chef policy-archive push
chef policy-cookbooks clean
chef policy-group delete
chef policy-revisions clean

Under the covers, chef policy-group delete could still invoke chef-policy group delete but that would be an internal implementation detail. It would make it easier for us to implement because it would be less changing of existing code. But I think the public contract is more important to cover in this design doc than the internal implementation of our command routing.

Thoughts?

Copy link
Author

Choose a reason for hiding this comment

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

From our conversation over zoom we have agreed that these commands look correct with the exception of chef-policy push-archive that will be removed and include into the chef-policy push command.

Copy link
Contributor

Choose a reason for hiding this comment

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

More context - I convinced myself that switching to a strictly enforced 'noun verb' syntax means we end up with a bunch of unnecessary nouns (policy-revision, policy-cookbooks, policy-groups, etc.) with 1 verb each.

Instead we talked over some of the existing commands and think the PR correctly documents them. The 'noun' in this case is always 'policy' because all these commands are all connected to Policyfiles.

The only tricky one is chef-policy clean cookbooks because it deletes cookbooks that are not referenced by a policyfile. Maybe this becomes an alias for something like chef cookbooks clean?

@afiune afiune requested a review from tyler-ball February 19, 2020 00:41
@afiune afiune force-pushed the design_proposal/chef_cli_catalog branch 2 times, most recently from 15d66f0 to cafcdf6 Compare February 19, 2020 00:44

### EXTRA: Present correct usage of sub-binaries

An additional task from this proposal is the use of an environment variable that
Copy link
Contributor

Choose a reason for hiding this comment

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

Talked with Salim about this - IMO, our first implementation should reference chef policy in all the help text. This makes it more confusing for our effortless developers who will directly invoke the chef-policy binary (without all the overhead of the entire Chef Workstation ecosystem). But those are internal users we can help.

In the future I could see us developing a Golang effortless executable that functions similarly to the chef golang executable. It allows us to start creating a custom workflow for them, leveraging tools under the covers that they do not care whether are written in Ruby or Golang. At that time I would introduce some kind of environment variable that would allow the help text to display chef policy or effortless policy.

@tyler-ball
Copy link
Contributor

Notes from group discussion: We want to add "auto-generated docs for tools/products" to the goals for this proposal.


#### Open questions for further discussion with UX:
* How do we want to present these sections? What is the right order?
* What verbiage should we use to express the relotation of commands? (`Use now` Vs `Moved to`)
Copy link
Contributor

Choose a reason for hiding this comment

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

now is unnecessary, can just say Use

@afiune afiune force-pushed the design_proposal/chef_cli_catalog branch from 03a5711 to 2b1bf20 Compare February 19, 2020 20:32
Salim Afiune added 9 commits February 19, 2020 13:34
Signed-off-by: Salim Afiune <afiune@chef.io>
Signed-off-by: Salim Afiune <afiune@chef.io>
Signed-off-by: Salim Afiune <afiune@chef.io>
Signed-off-by: Salim Afiune <afiune@chef.io>
Signed-off-by: Salim Afiune <afiune@chef.io>
Signed-off-by: Salim Afiune <afiune@chef.io>
Signed-off-by: Salim Afiune <afiune@chef.io>
Signed-off-by: Salim Afiune <afiune@chef.io>
Signed-off-by: Salim Afiune <afiune@chef.io>
@afiune afiune force-pushed the design_proposal/chef_cli_catalog branch 2 times, most recently from 2c366a1 to eb03de0 Compare February 19, 2020 20:36
Salim Afiune and others added 2 commits February 19, 2020 13:36
Signed-off-by: Salim Afiune <afiune@chef.io>
Co-Authored-By: Tyler Ball <tball@chef.io>
Signed-off-by: Salim Afiune <afiune@chef.io>
@afiune afiune force-pushed the design_proposal/chef_cli_catalog branch from eb03de0 to 3844264 Compare February 19, 2020 20:37
Salim Afiune added 2 commits February 20, 2020 08:32
Signed-off-by: Salim Afiune <afiune@chef.io>
Signed-off-by: Salim Afiune <afiune@chef.io>
@afiune afiune force-pushed the design_proposal/chef_cli_catalog branch from d81bf58 to 3d9c0df Compare February 20, 2020 19:52
Copy link
Contributor

@jonsmorrow jonsmorrow left a comment

Choose a reason for hiding this comment

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

There's additional spin off work to clarify the specific implementation, but this architecture and general direction looks good.

Signed-off-by: Salim Afiune <afiune@chef.io>
@afiune
Copy link
Author

afiune commented Feb 21, 2020

tenor-44698470

@afiune afiune merged commit 2a11487 into master Feb 21, 2020
@chef-expeditor chef-expeditor bot deleted the design_proposal/chef_cli_catalog branch February 21, 2020 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Adopted An issue that is being worked on. Triage: Confirmed Indicates and issue has been confirmed as described. Type: Design Proposal Community survey of a proposal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants