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

Revisit signature for the Configure method #18

Open
ogenstad opened this issue Apr 20, 2021 · 0 comments
Open

Revisit signature for the Configure method #18

ogenstad opened this issue Apr 20, 2021 · 0 comments

Comments

@ogenstad
Copy link
Member

The Configure method currently has this signature:

Configure(context.Context, []string) (string, error)

An error will only get returned if there's an issue with the SSH session or if the context has timed out. The returned string is just a concatenation of the output from the device during the configuration option. As mentioned in the intro post for Netrasp, this isn't ideal as configuration errors from the device more or less get ignored.

Netrasp should inspect the output from the network devices to determine the input is valid. What Netrasp does when an error occurs could be different for various platforms. For Cisco IOS, it could be that Netrasp stops applying configuration as soon as the output indicates config issues. Here it could be helpful for users if the return data contained a slice with used commands and the output from each, potentially making it easier to revert a bad change.

Things can work differently with other platforms that support staged configurations where the configurations are entered into a device and then applied with a commit command. For SROS, Junos, and IOS XR, it would make more sense to abort the configuration change as soon as an error in the config commands gets observed. For these platforms, an error could also occur after the commit command gets entered. When this happens, it should be clear for a user what caused the issue.

A better signature could be:
Configure(context.Context, []string) (ConfigResult, error)

ConfigResult would be a struct that can contain a slice of the applied configuration commands and output. It should also include output from the commit if applicable.

Error types would be configuration errors, along with commit errors (such as trying to apply a policy that doesn't exist, etc.)

Other thoughts for the Configuration method:

  • If a Configuration command gets aborted due to Context timeout, the actual device context will remain in an active configuration operation. The end-user would be responsible for aborting that configuration and exiting out to a root level (i.e., enable mode on IOS)
  • Some platforms could make sense to allow for options to Configure, such as commit-confirmed, etc.
@ogenstad ogenstad mentioned this issue Apr 21, 2021
ogenstad added a commit that referenced this issue Apr 22, 2021
This PR modified the method signature for Configure as described in #18.
It doesn't cover all features described in that issue. As it's a
breaking change for the early API it would be good to merge it sooner
rather than later and add to it once it's in place.
ogenstad added a commit that referenced this issue Apr 23, 2021
This PR modified the method signature for Configure as described in #18.
It doesn't cover all features described in that issue. As it's a
breaking change for the early API it would be good to merge it sooner
rather than later and add to it once it's in place.
ogenstad added a commit that referenced this issue Apr 23, 2021
This PR modified the method signature for Configure as described in #18.
It doesn't cover all features described in that issue. As it's a
breaking change for the early API it would be good to merge it sooner
rather than later and add to it once it's in place.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant