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

Capability detection for networker #626

Closed
wants to merge 28 commits into from
Closed

Capability detection for networker #626

wants to merge 28 commits into from

Conversation

themue
Copy link
Contributor

@themue themue commented Aug 28, 2014

In a first approach the decision on using the standard or the safe networker has been done based on the provider type. In case of all but MAAS the safe networker has been used. Now a new environment capability has been added, so that each provider can return if it requires a safe networker for the current machine. As this today depends on the machine (local provider dislikes the changes done by the networker on the bootstap node, because it's the host system) and if the machine has been manually provisioned.

Fixes #1354365.

Frank Mueller added 17 commits August 15, 2014 17:57
* master: (23 commits)
  Added tests for the help output.
  Using the setDoc for the command documentation.
  need to check for an error here
  added sent field to NewMetric factory
  worker/upgrader: use already downloaded tools if available
  rsyslog: updated documentation, simplified logrotate file rendering.
  rsyslog: comments for exported functions
  rsyslog: only render templates once, improve file check test
  rsyslog: check for logrotate files, updated comments
  rsyslog: logrotate tests
  rsyslog: logrotate comments and docs
  rsyslog: add logrotate for all-machines.log
  provider/maas: report arch matching tools in tests
  Update deps for utils package update
  Add extra logging
  corresponding changes to worker/
  Initial merge of 1.20 into master
  state/api/{keyupdater,networker}: use names.Tag
  unique strings for makemetrics
  added newmetrics function to testing factory
  ...
@@ -310,6 +311,20 @@ func openAPIState(agentConfig agent.Config, a Agent) (_ *api.State, _ *apiagent.
return st, entity, nil
}

// environCapability returns the capability of the environment

Choose a reason for hiding this comment

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

s/the capability/the capabilities/ ?

return mig.id
}

// IsManual returns the manually provisioning flag of the simulated machine.
Copy link

Choose a reason for hiding this comment

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

s/the manually provisioning flag.*$/whether the simulated machine was manually provisioned.

@dimitern
Copy link

dimitern commented Sep 5, 2014

A few more suggestions, it looks almost ready! Thanks!

Frank Mueller added 2 commits September 5, 2014 18:06
* master:
  Remove AddAdminUser and create the user when the environment is created.
  Fix tests
  Merge pull request #696 from wallyworld/keep-broken-part-2
  Remove DisplayName option
  Update names dependency.
  Remove the user checking for the keymanager.
  move the tests inline
  use lumberjack for log rolling
  juju/testing: don't remove admin user in JujuConnSuite
  Added comment
  Remove unneeded removes
  Add API func ShareEnvironment to client and server API:
bootstrapInstanceId instance.Id = "localhost"

// bootstrapMachineId is the id of the bootstrap machine and
// used to check if a safe networker is required.
Copy link

Choose a reason for hiding this comment

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

Please, rephrase like this:
// bootstrapMachineId is the id of the bootstrap machine.

@dimitern
Copy link

dimitern commented Sep 8, 2014

LGTM with the suggested minor changes: tech dept bug + TODO comment about making dummy provider's RequireSafeNetworker capability configurable in tests; renaming NewMachinerAPI to NewMachinerAPIV1 and leaving the older constructor named NewMachineAPIV0; fixing the suggested doc comments and other trivials.

@dimitern
Copy link

dimitern commented Sep 8, 2014

Ah, last but not least - rename this PR to remove the WIP prefix please.

@themue themue changed the title WIP Capability detection for networker Capability detection for networker Sep 8, 2014
// API v1 or higher, machines 0 and 1.
false, true, true, true,
false, true, true, true,
// API v0, machines 0 and 1.
Copy link
Contributor

Choose a reason for hiding this comment

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

Space before comment.

@themue themue closed this Sep 10, 2014
Tags []string
}

// GetMachinesResultsV0 holds the results of a MachinerAPIV1.GetMachines call.
Copy link
Contributor

Choose a reason for hiding this comment

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

typ0, should be: GetMachinesResultsV1.

@themue themue deleted the capability-detection-for-networker branch October 13, 2014 10:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants