11652: Merge branch 'master' into 11652-recursive-contents
authorTom Clegg <tom@curoverse.com>
Tue, 6 Jun 2017 17:53:36 +0000 (13:53 -0400)
committerTom Clegg <tom@curoverse.com>
Tue, 6 Jun 2017 17:53:36 +0000 (13:53 -0400)
39 files changed:
apps/workbench/app/assets/images/trash-icon.png [new file with mode: 0644]
apps/workbench/app/assets/javascripts/filterable.js
apps/workbench/app/assets/javascripts/select_modal.js
apps/workbench/app/assets/javascripts/selection.js.erb
apps/workbench/app/controllers/trash_items_controller.rb [new file with mode: 0644]
apps/workbench/app/models/arvados_base.rb
apps/workbench/app/models/arvados_resource_list.rb
apps/workbench/app/models/collection.rb
apps/workbench/app/views/application/_breadcrumbs.html.erb
apps/workbench/app/views/trash_items/_create_new_object_button.html.erb [new file with mode: 0644]
apps/workbench/app/views/trash_items/_show_recent_trash.html.erb [new file with mode: 0644]
apps/workbench/app/views/trash_items/_show_trash_rows.html.erb [new file with mode: 0644]
apps/workbench/app/views/trash_items/_untrash_item.html.erb [new file with mode: 0644]
apps/workbench/app/views/trash_items/index.html.erb [new file with mode: 0644]
apps/workbench/app/views/trash_items/untrash_items.js.erb [new file with mode: 0644]
apps/workbench/app/views/work_units/_show_all_processes.html.erb
apps/workbench/config/routes.rb
apps/workbench/test/integration/trash_test.rb [new file with mode: 0644]
build/run-tests.sh
sdk/go/arvados/client.go
sdk/go/arvadosclient/arvadosclient.go
sdk/go/keepclient/discover.go
sdk/go/keepclient/discover_test.go
sdk/go/keepclient/keepclient.go
sdk/go/keepclient/keepclient_test.go
sdk/go/keepclient/support.go
services/api/app/controllers/arvados/v1/collections_controller.rb
services/api/app/controllers/arvados/v1/keep_services_controller.rb
services/api/config/routes.rb
services/api/test/fixtures/collections.yml
services/api/test/functional/arvados/v1/collections_controller_test.rb
services/api/test/functional/arvados/v1/keep_services_controller_test.rb
services/keep-web/handler.go
services/keepproxy/keepproxy.go
services/keepstore/pull_worker_integration_test.go
tools/keep-block-check/keep-block-check.go
tools/keep-block-check/keep-block-check_test.go
tools/keep-rsync/keep-rsync.go
tools/keep-rsync/keep-rsync_test.go

diff --git a/apps/workbench/app/assets/images/trash-icon.png b/apps/workbench/app/assets/images/trash-icon.png
new file mode 100644 (file)
index 0000000..5c26c24
Binary files /dev/null and b/apps/workbench/app/assets/images/trash-icon.png differ
index 5f6370c2905171bda848622f9d2e6e638d33ad9b..e3639d0f2bdefea6d72da8b0fd6cd34bbe47f4c2 100644 (file)
@@ -57,6 +57,7 @@ function updateFilterableQueryNow($target) {
     } else {
       params.filters = [['any', '@@', newquery.trim().concat(':*')]];
     }
+    $(".modal-dialog-preview-pane").html("");
     $target.data('infinite-content-params-filterable', params);
     $target.data('filterable-query', newquery);
 }
index 17b334eb643438631eb35c3b8ffa31d04d9c2d30..d31cb45dbaf8bcd9cb2ac1ec65a1ab2a8b3c773d 100644 (file)
@@ -120,6 +120,7 @@ $(document).on('click', '.selectable', function() {
                   'project_uuid': project_uuid
                  };
     }
+    $(".modal-dialog-preview-pane").html("");
     // Use current selection as dropdown button label
     $(this).
         closest('.dropdown-menu').
index f60bef7ddb432cda137c935f395b1a46abe997ad..a8e2738fe0ed69c8f4850b1bb9fef8bb3b1b1ab2 100644 (file)
@@ -81,6 +81,10 @@ function enable_disable_selection_actions() {
         toggleClass('disabled',
                     ($checked.length < 0) ||
                     !($checked.length > 0 && collection_lock_classes && collection_lock_classes.indexOf("fa-unlock") !=-1));
+    $('[data-selection-action=untrash-selected-items]', $container).
+        closest('li').
+        toggleClass('disabled',
+                    ($checked.length < 1));
 }
 
 $(document).
diff --git a/apps/workbench/app/controllers/trash_items_controller.rb b/apps/workbench/app/controllers/trash_items_controller.rb
new file mode 100644 (file)
index 0000000..5f91a60
--- /dev/null
@@ -0,0 +1,88 @@
+class TrashItemsController < ApplicationController
+  def model_class
+    Collection
+  end
+
+  def index_pane_list
+    %w(Recent_trash)
+  end
+
+  def find_objects_for_index
+    # If it's not the index rows partial display, just return
+    # The /index request will again be invoked to display the
+    # partial at which time, we will be using the objects found.
+    return if !params[:partial]
+
+    trashed_items
+
+    if @objects.any?
+      @objects = @objects.sort_by { |obj| obj.trash_at }.reverse
+      @next_page_filters = next_page_filters('<=')
+      @next_page_href = url_for(partial: :trash_rows,
+                                filters: @next_page_filters.to_json)
+    else
+      @next_page_href = nil
+    end
+  end
+
+  def next_page_href with_params={}
+    @next_page_href
+  end
+
+  def next_page_filters nextpage_operator
+    next_page_filters = @filters.reject do |attr, op, val|
+      (attr == 'trash_at' and op == nextpage_operator) or
+      (attr == 'uuid' and op == 'not in')
+    end
+
+    if @objects.any?
+      last_trash_at = @objects.last.trash_at
+
+      last_uuids = []
+      @objects.each do |obj|
+        last_uuids << obj.uuid if obj.trash_at.eql?(last_trash_at)
+      end
+
+      next_page_filters += [['trash_at', nextpage_operator, last_trash_at]]
+      next_page_filters += [['uuid', 'not in', last_uuids]]
+    end
+
+    next_page_filters
+  end
+
+  def trashed_items
+    # API server index doesn't return manifest_text by default, but our
+    # callers want it unless otherwise specified.
+    @select ||= Collection.columns.map(&:name)
+    limit = if params[:limit] then params[:limit].to_i else 100 end
+    offset = if params[:offset] then params[:offset].to_i else 0 end
+
+    base_search = Collection.select(@select).include_trash(true).where(is_trashed: true)
+    base_search = base_search.filter(params[:filters]) if params[:filters]
+
+    if params[:search].andand.length.andand > 0
+      tags = Link.where(any: ['contains', params[:search]])
+      base_search = base_search.limit(limit).offset(offset)
+      @objects = (base_search.where(uuid: tags.collect(&:head_uuid)) |
+                  base_search.where(any: ['contains', params[:search]])).
+                  uniq { |c| c.uuid }
+    else
+      @objects = base_search.limit(limit).offset(offset)
+    end
+  end
+
+  def untrash_items
+    @untrashed_uuids = []
+
+    updates = {trash_at: nil}
+
+    Collection.include_trash(1).where(uuid: params[:selection]).each do |c|
+      c.untrash
+      @untrashed_uuids << c.uuid
+    end
+
+    respond_to do |format|
+      format.js
+    end
+  end
+end
index 5d6a4c94b9bebb581d771ce4b0557f3ba828e119..f06193c3ecfaeb307ae0a2a86009270b9a4b266c 100644 (file)
@@ -152,6 +152,10 @@ class ArvadosBase < ActiveRecord::Base
     ArvadosResourceList.new(self).distinct(*args)
   end
 
+  def self.include_trash(*args)
+    ArvadosResourceList.new(self).include_trash(*args)
+  end
+
   def self.eager(*args)
     ArvadosResourceList.new(self).eager(*args)
   end
index 35dcde38da24cabc5290e24e0a99131f467181b4..8ae48d82443587f2d042d70327d802ed8f466150 100644 (file)
@@ -21,6 +21,11 @@ class ArvadosResourceList
     self
   end
 
+  def include_trash(option=nil)
+    @include_trash = option
+    self
+  end
+
   def limit(max_results)
     if not max_results.nil? and not max_results.is_a? Integer
       raise ArgumentError("argument to limit() must be an Integer or nil")
@@ -192,6 +197,7 @@ class ArvadosResourceList
     api_params[:order] = @orderby_spec if @orderby_spec
     api_params[:filters] = @filters if @filters
     api_params[:distinct] = @distinct if @distinct
+    api_params[:include_trash] = @include_trash if @include_trash
     if @fetch_multiple_pages
       # Default limit to (effectively) api server's MAX_LIMIT
       api_params[:limit] = 2**(0.size*8 - 1) - 1
index ea81ad8c0a7588edc00062585d99ba9fa116035f..305ea015306fe898c97bd16b443a13d9ff230781 100644 (file)
@@ -98,4 +98,7 @@ class Collection < ArvadosBase
     [ 'description' ]
   end
 
+  def untrash
+    arvados_api_client.api(self.class, "/#{self.uuid}/untrash", {})
+  end
 end
index 3ef2aec17c35575013dd0be2ca51afe95953c8ba..1ead5787b8b54363bec746003a8aa878d7f356b9 100644 (file)
             <% end %>
           <% end %>
         </ul>
+        <ul class="nav navbar-nav navbar-right">
+          <li>
+            <a href="/trash">
+              <%= image_tag("trash-icon.png", size: "20x20" ) %> Trash
+            </a>
+          </li>
+        </ul>
       </nav>
diff --git a/apps/workbench/app/views/trash_items/_create_new_object_button.html.erb b/apps/workbench/app/views/trash_items/_create_new_object_button.html.erb
new file mode 100644 (file)
index 0000000..2ba9e1a
--- /dev/null
@@ -0,0 +1 @@
+<%# There is no such thing %>
diff --git a/apps/workbench/app/views/trash_items/_show_recent_trash.html.erb b/apps/workbench/app/views/trash_items/_show_recent_trash.html.erb
new file mode 100644 (file)
index 0000000..4d9be07
--- /dev/null
@@ -0,0 +1,54 @@
+<div class="container selection-action-container" style="width: 100%">
+  <div class="col-md-2 pull-left">
+    <div class="btn-group btn-group-sm">
+      <button type="button" class="btn btn-default dropdown-toggle" data-toggle="dropdown">Selection... <span class="caret"></span></button>
+      <ul class="dropdown-menu" role="menu">
+        <li><%= link_to "Un-trash selected items", '#',
+                method: :post,
+                remote: true,
+                'id' => 'untrash_selected_items',
+                'data-href' => untrash_items_trash_items_path,
+                'data-selection-param-name' => 'selection[]',
+                'data-selection-action' => 'untrash-selected-items',
+                'data-toggle' => 'dropdown'
+          %></li>
+      </ul>
+    </div>
+  </div>
+  <div class="col-md-4 pull-right">
+    <input type="text" class="form-control filterable-control recent-trash-items"
+           placeholder="Search trash"
+           data-filterable-target="#recent-trash-items"
+           value="<%= params[:search] %>" />
+  </div>
+
+  <div>
+    <table id="trash-index" class="topalign table table-condensed table-fixedlayout">
+      <colgroup>
+        <col width="5%" />
+        <col width="20%" />
+        <col width="15%" />
+        <col width="15%" />
+        <col width="10%" />
+        <col width="30%" />
+        <col width="5%" />
+      </colgroup>
+
+      <thead>
+        <tr class="contain-align-left">
+          <th></th>
+          <th>Name</th>
+          <th>Trashed at</th>
+          <th>Created at</th>
+          <th>Owner</th>
+          <th>Contents</th>
+          <th></th>
+        </tr>
+      </thead>
+
+      <tbody data-infinite-scroller="#recent-trash-items" id="recent-trash-items"
+        data-infinite-content-href="<%= url_for partial: :trash_rows %>" >
+      </tbody>
+    </table>
+  </div>
+</div>
diff --git a/apps/workbench/app/views/trash_items/_show_trash_rows.html.erb b/apps/workbench/app/views/trash_items/_show_trash_rows.html.erb
new file mode 100644 (file)
index 0000000..4cc16f6
--- /dev/null
@@ -0,0 +1,32 @@
+<% @objects.each do |obj| %>
+    <tr data-object-uuid="<%= obj.uuid %>" data-kind="<%= obj.kind %>" >
+      <td>
+        <% if obj.editable? %>
+          <%= check_box_tag 'uuids[]', obj.uuid, false, :class => 'persistent-selection' %>
+        <% end %>
+      </td>
+      <td>
+        <%= if !obj.name.blank? then obj.name else obj.uuid end %>
+      <td>
+        <%= render_localized_date(obj.trash_at) if obj.trash_at %>
+      <td>
+        <%= render_localized_date(obj.created_at) if obj.created_at %>
+      </td>
+      <td>
+        <%= link_to_if_arvados_object obj.owner_uuid, friendly_name: true %>
+      </td>
+      <td>
+        <% for i in (0..[2, obj.files.length-1].min) %>
+          <% file = obj.files[i] %>
+          <% file_path = "#{file[0]}/#{file[1]}" %>
+          <%= file_path %><br />
+        <% end %>
+        <% if obj.files.length > 3 %>
+          <%= "(#{obj.files.length-3} more files)" %>
+        <% end %>
+      </td>
+      <td>
+        <%= render partial: 'untrash_item', locals: {object:obj} %>
+      </td>
+    </tr>
+<% end %>
diff --git a/apps/workbench/app/views/trash_items/_untrash_item.html.erb b/apps/workbench/app/views/trash_items/_untrash_item.html.erb
new file mode 100644 (file)
index 0000000..8732531
--- /dev/null
@@ -0,0 +1,7 @@
+<% if object.editable? %>
+  <% msg = "Untrash '" + if !object.name.blank? then object.name else object.uuid end + "'?" %>
+  <%= link_to({action: 'untrash_items', selection: [object.uuid]}, remote: true, method: :post,
+      title: "Untrash", data: {confirm: msg}) do %>
+    <i class="fa fa-fw fa-recycle"></i>
+  <% end %>
+<% end %>
diff --git a/apps/workbench/app/views/trash_items/index.html.erb b/apps/workbench/app/views/trash_items/index.html.erb
new file mode 100644 (file)
index 0000000..5f5bc83
--- /dev/null
@@ -0,0 +1 @@
+<%= render file: 'application/index.html.erb', locals: local_assigns %>
diff --git a/apps/workbench/app/views/trash_items/untrash_items.js.erb b/apps/workbench/app/views/trash_items/untrash_items.js.erb
new file mode 100644 (file)
index 0000000..3d26658
--- /dev/null
@@ -0,0 +1,5 @@
+<% @untrashed_uuids.each do |uuid| %>
+       $('[data-object-uuid=<%= uuid %>]').hide('slow', function() {
+           $(this).remove();
+       });
+<% end %>
index ea178438cfef1fe912bed6f22ec5a5f54bb9b6b2..0fc1ef625f4d96dbd7470abbbe13a7c439dd150a 100644 (file)
@@ -1,4 +1,4 @@
-<div class="container">
+<div class="container" style="width: 100%">
   <div class="row">
     <div class="pull-right">
       <input type="text" class="form-control filterable-control recent-all-processes-filterable-control"
index 0eef73f8ae3d0116ba96ccf8f93f6663fc4204e8..badb471d64ae666924fb06a21a408237bbed3f26 100644 (file)
@@ -108,6 +108,11 @@ ArvadosWorkbench::Application.routes.draw do
 
   resources :workflows
 
+  get "trash" => 'trash_items#index', :as => :trash
+  resources :trash_items do
+    post 'untrash_items', on: :collection
+  end
+
   post 'actions' => 'actions#post'
   get 'actions' => 'actions#show'
   get 'websockets' => 'websocket#index'
diff --git a/apps/workbench/test/integration/trash_test.rb b/apps/workbench/test/integration/trash_test.rb
new file mode 100644 (file)
index 0000000..6cac1be
--- /dev/null
@@ -0,0 +1,87 @@
+require 'integration_helper'
+
+class TrashTest < ActionDispatch::IntegrationTest
+  setup do
+    need_javascript
+  end
+
+  test "trash page" do
+    deleted = api_fixture('collections')['deleted_on_next_sweep']
+    expired1 = api_fixture('collections')['unique_expired_collection']
+    expired2 = api_fixture('collections')['unique_expired_collection2']
+
+    # visit trash page
+    visit page_with_token('active', "/trash")
+
+    assert_text deleted['name']
+    assert_text expired1['name']
+    assert_text expired2['name']
+    assert_no_text 'foo_file'
+
+    # Un-trash one item using selection dropdown
+    within('tr', text: deleted['name']) do
+      first('input').click
+    end
+
+    click_button 'Selection...'
+    within('.selection-action-container') do
+      click_link 'Un-trash selected items'
+    end
+
+    wait_for_ajax
+
+    assert_text expired1['name']      # this should still be there
+    assert_no_text deleted['name']    # this should no longer be here
+
+    # expired2 is not editable by me; checkbox and recycle button shouldn't be offered
+    within('tr', text: expired2['name']) do
+      assert_nil first('input')
+      assert_nil first('.fa-recycle')
+    end
+
+    # Un-trash another item using the recycle button
+    within('tr', text: expired1['name']) do
+      first('.fa-recycle').click
+      accept_alert
+    end
+
+    wait_for_ajax
+
+    assert_text expired2['name']
+    assert_no_text expired1['name']
+
+    # verify that the two un-trashed items are now shown in /collections page
+    visit page_with_token('active', "/collections")
+    assert_text deleted['uuid']
+    assert_text expired1['uuid']
+    assert_no_text expired2['uuid']
+  end
+
+  test "trash page with search" do
+    deleted = api_fixture('collections')['deleted_on_next_sweep']
+    expired = api_fixture('collections')['unique_expired_collection2']
+
+    visit page_with_token('active', "/trash")
+
+    assert_text deleted['name']
+    assert_text expired['name']
+
+    page.find_field('Search trash').set 'expired'
+
+    assert_text expired['name']
+    assert_no_text deleted['name']
+
+    click_button 'Selection...'
+    within('.selection-action-container') do
+      assert_selector 'li.disabled', text: 'Un-trash selected items'
+    end
+
+    first('input').click
+
+    click_button 'Selection...'
+    within('.selection-action-container') do
+      assert_selector 'li', text: 'Un-trash selected items'
+      assert_selector 'li.disabled', text: 'Un-trash selected items'
+    end
+  end
+end
index b6a93d471211a41a3d97c6aa8bbe11e3b033e130..dffbe32e8ab9c89d6cecf02dd445c8314d2560ad 100755 (executable)
@@ -567,7 +567,7 @@ do_test_once() {
         # mode makes Go show the wrong line numbers when reporting
         # compilation errors.
         go get -t "git.curoverse.com/arvados.git/$1" && \
-            cd "$WORKSPACE/$1" && \
+            cd "$GOPATH/src/git.curoverse.com/arvados.git/$1" && \
             [[ -z "$(gofmt -e -d . | tee /dev/stderr)" ]] && \
             if [[ -n "${testargs[$1]}" ]]
         then
index 9691e7a07e475668cc73a80dffba466addb3b4ef..d7eb811b8a7a26a08931b07b9c66fcfec83d78a3 100644 (file)
@@ -6,6 +6,7 @@ import (
        "fmt"
        "io"
        "io/ioutil"
+       "log"
        "math"
        "net/http"
        "net/url"
@@ -63,13 +64,25 @@ var DefaultSecureClient = &http.Client{
 // ARVADOS_API_* environment variables.
 func NewClientFromEnv() *Client {
        var svcs []string
-       if s := os.Getenv("ARVADOS_KEEP_SERVICES"); s != "" {
-               svcs = strings.Split(s, " ")
+       for _, s := range strings.Split(os.Getenv("ARVADOS_KEEP_SERVICES"), " ") {
+               if s == "" {
+                       continue
+               } else if u, err := url.Parse(s); err != nil {
+                       log.Printf("ARVADOS_KEEP_SERVICES: %q: %s", s, err)
+               } else if !u.IsAbs() {
+                       log.Printf("ARVADOS_KEEP_SERVICES: %q: not an absolute URI", s)
+               } else {
+                       svcs = append(svcs, s)
+               }
+       }
+       var insecure bool
+       if s := strings.ToLower(os.Getenv("ARVADOS_API_HOST_INSECURE")); s == "1" || s == "yes" || s == "true" {
+               insecure = true
        }
        return &Client{
                APIHost:         os.Getenv("ARVADOS_API_HOST"),
                AuthToken:       os.Getenv("ARVADOS_API_TOKEN"),
-               Insecure:        os.Getenv("ARVADOS_API_HOST_INSECURE") != "",
+               Insecure:        insecure,
                KeepServiceURIs: svcs,
        }
 }
index af7f028e0725635fefbae6b880d1506e788b2dd1..4cfda94581518fd9360ebc4f3268e231893797c7 100644 (file)
@@ -74,6 +74,13 @@ func (e APIServerError) Error() string {
        }
 }
 
+// StringBool tests whether s is suggestive of true. It returns true
+// if s is a mixed/uppoer/lower-case variant of "1", "yes", or "true".
+func StringBool(s string) bool {
+       s = strings.ToLower(s)
+       return s == "1" || s == "yes" || s == "true"
+}
+
 // Helper type so we don't have to write out 'map[string]interface{}' every time.
 type Dict map[string]interface{}
 
@@ -163,6 +170,7 @@ func New(c *arvados.Client) (*ArvadosClient, error) {
                        TLSClientConfig: MakeTLSConfig(c.Insecure)}},
                External:          false,
                Retries:           2,
+               KeepServiceURIs:   c.KeepServiceURIs,
                lastClosedIdlesAt: time.Now(),
        }
 
@@ -174,42 +182,12 @@ func New(c *arvados.Client) (*ArvadosClient, error) {
 // ARVADOS_API_HOST_INSECURE, ARVADOS_EXTERNAL_CLIENT, and
 // ARVADOS_KEEP_SERVICES.
 func MakeArvadosClient() (ac *ArvadosClient, err error) {
-       var matchTrue = regexp.MustCompile("^(?i:1|yes|true)$")
-       insecure := matchTrue.MatchString(os.Getenv("ARVADOS_API_HOST_INSECURE"))
-       external := matchTrue.MatchString(os.Getenv("ARVADOS_EXTERNAL_CLIENT"))
-
-       ac = &ArvadosClient{
-               Scheme:      "https",
-               ApiServer:   os.Getenv("ARVADOS_API_HOST"),
-               ApiToken:    os.Getenv("ARVADOS_API_TOKEN"),
-               ApiInsecure: insecure,
-               Client: &http.Client{Transport: &http.Transport{
-                       TLSClientConfig: MakeTLSConfig(insecure)}},
-               External: external,
-               Retries:  2}
-
-       for _, s := range strings.Split(os.Getenv("ARVADOS_KEEP_SERVICES"), " ") {
-               if s == "" {
-                       continue
-               }
-               if u, err := url.Parse(s); err != nil {
-                       return ac, fmt.Errorf("ARVADOS_KEEP_SERVICES: %q: %s", s, err)
-               } else if !u.IsAbs() {
-                       return ac, fmt.Errorf("ARVADOS_KEEP_SERVICES: %q: not an absolute URI", s)
-               }
-               ac.KeepServiceURIs = append(ac.KeepServiceURIs, s)
-       }
-
-       if ac.ApiServer == "" {
-               return ac, MissingArvadosApiHost
-       }
-       if ac.ApiToken == "" {
-               return ac, MissingArvadosApiToken
+       ac, err = New(arvados.NewClientFromEnv())
+       if err != nil {
+               return
        }
-
-       ac.lastClosedIdlesAt = time.Now()
-
-       return ac, err
+       ac.External = StringBool(os.Getenv("ARVADOS_EXTERNAL_CLIENT"))
+       return
 }
 
 // CallRaw is the same as Call() but returns a Reader that reads the
index 8889c4bedae23f52d62667ba5b8747ed11c3f67b..e2cd329fc4c22ccfdb73ff6099ce8c95f6f28d16 100644 (file)
@@ -6,99 +6,165 @@ import (
        "log"
        "os"
        "os/signal"
-       "reflect"
        "strings"
+       "sync"
        "syscall"
        "time"
+
+       "git.curoverse.com/arvados.git/sdk/go/arvadosclient"
 )
 
-// DiscoverKeepServers gets list of available keep services from the
-// API server.
-//
-// If a list of services is provided in the arvadosclient (e.g., from
-// an environment variable or local config), that list is used
-// instead.
-func (this *KeepClient) DiscoverKeepServers() error {
-       if this.Arvados.KeepServiceURIs != nil {
-               this.foundNonDiskSvc = true
-               this.replicasPerService = 0
-               roots := make(map[string]string)
-               for i, uri := range this.Arvados.KeepServiceURIs {
-                       roots[fmt.Sprintf("00000-bi6l4-%015d", i)] = uri
-               }
-               this.SetServiceRoots(roots, roots, roots)
-               return nil
+// ClearCache clears the Keep service discovery cache.
+func RefreshServiceDiscovery() {
+       svcListCacheMtx.Lock()
+       defer svcListCacheMtx.Unlock()
+       for _, ent := range svcListCache {
+               ent.clear <- struct{}{}
        }
+}
 
-       // ArvadosClient did not provide a services list. Ask API
-       // server for a list of accessible services.
-       var list svcList
-       err := this.Arvados.Call("GET", "keep_services", "", "accessible", nil, &list)
-       if err != nil {
-               return err
+// ClearCacheOnSIGHUP installs a signal handler that calls
+// ClearCache when SIGHUP is received.
+func RefreshServiceDiscoveryOnSIGHUP() {
+       svcListCacheMtx.Lock()
+       defer svcListCacheMtx.Unlock()
+       if svcListCacheSignal != nil {
+               return
        }
-       return this.loadKeepServers(list)
+       svcListCacheSignal = make(chan os.Signal, 1)
+       signal.Notify(svcListCacheSignal, syscall.SIGHUP)
+       go func() {
+               for range svcListCacheSignal {
+                       RefreshServiceDiscovery()
+               }
+       }()
 }
 
-// LoadKeepServicesFromJSON gets list of available keep services from given JSON
-func (this *KeepClient) LoadKeepServicesFromJSON(services string) error {
-       var list svcList
-
-       // Load keep services from given json
-       dec := json.NewDecoder(strings.NewReader(services))
-       if err := dec.Decode(&list); err != nil {
-               return err
-       }
+var (
+       svcListCache       = map[string]cachedSvcList{}
+       svcListCacheSignal chan os.Signal
+       svcListCacheMtx    sync.Mutex
+)
 
-       return this.loadKeepServers(list)
+type cachedSvcList struct {
+       arv    *arvadosclient.ArvadosClient
+       latest chan svcList
+       clear  chan struct{}
 }
 
-// RefreshServices calls DiscoverKeepServers to refresh the keep
-// service list on SIGHUP; when the given interval has elapsed since
-// the last refresh; and (if the last refresh failed) the given
-// errInterval has elapsed.
-func (kc *KeepClient) RefreshServices(interval, errInterval time.Duration) {
-       var previousRoots = []map[string]string{}
-
-       timer := time.NewTimer(interval)
-       gotHUP := make(chan os.Signal, 1)
-       signal.Notify(gotHUP, syscall.SIGHUP)
+// Check for new services list every few minutes. Send the latest list
+// to the "latest" channel as needed.
+func (ent *cachedSvcList) poll() {
+       wakeup := make(chan struct{})
+
+       replace := make(chan svcList)
+       go func() {
+               wakeup <- struct{}{}
+               current := <-replace
+               for {
+                       select {
+                       case <-ent.clear:
+                               wakeup <- struct{}{}
+                               // Wait here for the next success, in
+                               // order to avoid returning stale
+                               // results on the "latest" channel.
+                               current = <-replace
+                       case current = <-replace:
+                       case ent.latest <- current:
+                       }
+               }
+       }()
 
+       okDelay := 5 * time.Minute
+       errDelay := 3 * time.Second
+       timer := time.NewTimer(okDelay)
        for {
                select {
-               case <-gotHUP:
                case <-timer.C:
+               case <-wakeup:
+                       if !timer.Stop() {
+                               // Lost race stopping timer; skip extra firing
+                               <-timer.C
+                       }
                }
-               timer.Reset(interval)
-
-               if err := kc.DiscoverKeepServers(); err != nil {
-                       log.Printf("WARNING: Error retrieving services list: %v (retrying in %v)", err, errInterval)
-                       timer.Reset(errInterval)
+               var next svcList
+               err := ent.arv.Call("GET", "keep_services", "", "accessible", nil, &next)
+               if err != nil {
+                       log.Printf("WARNING: Error retrieving services list: %v (retrying in %v)", err, errDelay)
+                       timer.Reset(errDelay)
                        continue
                }
-               newRoots := []map[string]string{kc.LocalRoots(), kc.GatewayRoots()}
+               replace <- next
+               timer.Reset(okDelay)
+       }
+}
+
+// discoverServices gets the list of available keep services from
+// the API server.
+//
+// If a list of services is provided in the arvadosclient (e.g., from
+// an environment variable or local config), that list is used
+// instead.
+//
+// If an API call is made, the result is cached for 5 minutes or until
+// ClearCache() is called, and during this interval it is reused by
+// other KeepClients that use the same API server host.
+func (kc *KeepClient) discoverServices() error {
+       if kc.disableDiscovery {
+               return nil
+       }
 
-               if !reflect.DeepEqual(previousRoots, newRoots) {
-                       DebugPrintf("DEBUG: Updated services list: locals %v gateways %v", newRoots[0], newRoots[1])
-                       previousRoots = newRoots
+       if kc.Arvados.KeepServiceURIs != nil {
+               kc.disableDiscovery = true
+               kc.foundNonDiskSvc = true
+               kc.replicasPerService = 0
+               roots := make(map[string]string)
+               for i, uri := range kc.Arvados.KeepServiceURIs {
+                       roots[fmt.Sprintf("00000-bi6l4-%015d", i)] = uri
                }
+               kc.setServiceRoots(roots, roots, roots)
+               return nil
+       }
 
-               if len(newRoots[0]) == 0 {
-                       log.Printf("WARNING: No local services (retrying in %v)", errInterval)
-                       timer.Reset(errInterval)
+       svcListCacheMtx.Lock()
+       cacheEnt, ok := svcListCache[kc.Arvados.ApiServer]
+       if !ok {
+               arv := *kc.Arvados
+               cacheEnt = cachedSvcList{
+                       latest: make(chan svcList),
+                       clear:  make(chan struct{}),
+                       arv:    &arv,
                }
+               go cacheEnt.poll()
+               svcListCache[kc.Arvados.ApiServer] = cacheEnt
+       }
+       svcListCacheMtx.Unlock()
+
+       return kc.loadKeepServers(<-cacheEnt.latest)
+}
+
+// LoadKeepServicesFromJSON gets list of available keep services from
+// given JSON and disables automatic service discovery.
+func (kc *KeepClient) LoadKeepServicesFromJSON(services string) error {
+       kc.disableDiscovery = true
+
+       var list svcList
+       dec := json.NewDecoder(strings.NewReader(services))
+       if err := dec.Decode(&list); err != nil {
+               return err
        }
+
+       return kc.loadKeepServers(list)
 }
 
-// loadKeepServers
-func (this *KeepClient) loadKeepServers(list svcList) error {
+func (kc *KeepClient) loadKeepServers(list svcList) error {
        listed := make(map[string]bool)
        localRoots := make(map[string]string)
        gatewayRoots := make(map[string]string)
        writableLocalRoots := make(map[string]string)
 
        // replicasPerService is 1 for disks; unknown or unlimited otherwise
-       this.replicasPerService = 1
+       kc.replicasPerService = 1
 
        for _, service := range list.Items {
                scheme := "http"
@@ -117,12 +183,12 @@ func (this *KeepClient) loadKeepServers(list svcList) error {
                if service.ReadOnly == false {
                        writableLocalRoots[service.Uuid] = url
                        if service.SvcType != "disk" {
-                               this.replicasPerService = 0
+                               kc.replicasPerService = 0
                        }
                }
 
                if service.SvcType != "disk" {
-                       this.foundNonDiskSvc = true
+                       kc.foundNonDiskSvc = true
                }
 
                // Gateway services are only used when specified by
@@ -133,6 +199,6 @@ func (this *KeepClient) loadKeepServers(list svcList) error {
                gatewayRoots[service.Uuid] = url
        }
 
-       this.SetServiceRoots(localRoots, writableLocalRoots, gatewayRoots)
+       kc.setServiceRoots(localRoots, writableLocalRoots, gatewayRoots)
        return nil
 }
index 379d44c820aec0e0b88d84b3c52e5fd316480844..4065ce342e43dfaa5172ca48f4a3ec3282045296 100644 (file)
@@ -3,28 +3,15 @@ package keepclient
 import (
        "crypto/md5"
        "fmt"
-       "gopkg.in/check.v1"
        "net/http"
        "os"
-       "time"
+
+       "gopkg.in/check.v1"
 
        "git.curoverse.com/arvados.git/sdk/go/arvadosclient"
        "git.curoverse.com/arvados.git/sdk/go/arvadostest"
 )
 
-func ExampleKeepClient_RefreshServices() {
-       arv, err := arvadosclient.MakeArvadosClient()
-       if err != nil {
-               panic(err)
-       }
-       kc, err := MakeKeepClient(arv)
-       if err != nil {
-               panic(err)
-       }
-       go kc.RefreshServices(5*time.Minute, 3*time.Second)
-       fmt.Printf("LocalRoots: %#v\n", kc.LocalRoots())
-}
-
 func (s *ServerRequiredSuite) TestOverrideDiscovery(c *check.C) {
        defer os.Setenv("ARVADOS_KEEP_SERVICES", "")
 
index 76ea17517fc6d2b59e0c96e2a33fb3561fcd38df..029c6ee7f3a5834a8af149b64917aff2d5dc4bcb 100644 (file)
@@ -88,9 +88,9 @@ type HTTPClient interface {
 type KeepClient struct {
        Arvados            *arvadosclient.ArvadosClient
        Want_replicas      int
-       localRoots         *map[string]string
-       writableLocalRoots *map[string]string
-       gatewayRoots       *map[string]string
+       localRoots         map[string]string
+       writableLocalRoots map[string]string
+       gatewayRoots       map[string]string
        lock               sync.RWMutex
        HTTPClient         HTTPClient
        Retries            int
@@ -101,6 +101,9 @@ type KeepClient struct {
 
        // Any non-disk typed services found in the list of keepservers?
        foundNonDiskSvc bool
+
+       // Disable automatic discovery of keep services
+       disableDiscovery bool
 }
 
 // MakeKeepClient creates a new KeepClient, calls
@@ -108,12 +111,11 @@ type KeepClient struct {
 // use.
 func MakeKeepClient(arv *arvadosclient.ArvadosClient) (*KeepClient, error) {
        kc := New(arv)
-       return kc, kc.DiscoverKeepServers()
+       return kc, kc.discoverServices()
 }
 
-// New creates a new KeepClient. The caller must call
-// DiscoverKeepServers() before using the returned client to read or
-// write data.
+// New creates a new KeepClient. Service discovery will occur on the
+// next read/write operation.
 func New(arv *arvadosclient.ArvadosClient) *KeepClient {
        defaultReplicationLevel := 2
        value, err := arv.Discovery("defaultCollectionReplication")
@@ -349,55 +351,47 @@ func (kc *KeepClient) GetIndex(keepServiceUUID, prefix string) (io.Reader, error
 // LocalRoots() returns the map of local (i.e., disk and proxy) Keep
 // services: uuid -> baseURI.
 func (kc *KeepClient) LocalRoots() map[string]string {
+       kc.discoverServices()
        kc.lock.RLock()
        defer kc.lock.RUnlock()
-       return *kc.localRoots
+       return kc.localRoots
 }
 
 // GatewayRoots() returns the map of Keep remote gateway services:
 // uuid -> baseURI.
 func (kc *KeepClient) GatewayRoots() map[string]string {
+       kc.discoverServices()
        kc.lock.RLock()
        defer kc.lock.RUnlock()
-       return *kc.gatewayRoots
+       return kc.gatewayRoots
 }
 
 // WritableLocalRoots() returns the map of writable local Keep services:
 // uuid -> baseURI.
 func (kc *KeepClient) WritableLocalRoots() map[string]string {
+       kc.discoverServices()
        kc.lock.RLock()
        defer kc.lock.RUnlock()
-       return *kc.writableLocalRoots
+       return kc.writableLocalRoots
 }
 
-// SetServiceRoots updates the localRoots and gatewayRoots maps,
-// without risk of disrupting operations that are already in progress.
+// SetServiceRoots disables service discovery and updates the
+// localRoots and gatewayRoots maps, without disrupting operations
+// that are already in progress.
 //
-// The KeepClient makes its own copy of the supplied maps, so the
-// caller can reuse/modify them after SetServiceRoots returns, but
-// they should not be modified by any other goroutine while
-// SetServiceRoots is running.
-func (kc *KeepClient) SetServiceRoots(newLocals, newWritableLocals, newGateways map[string]string) {
-       locals := make(map[string]string)
-       for uuid, root := range newLocals {
-               locals[uuid] = root
-       }
-
-       writables := make(map[string]string)
-       for uuid, root := range newWritableLocals {
-               writables[uuid] = root
-       }
-
-       gateways := make(map[string]string)
-       for uuid, root := range newGateways {
-               gateways[uuid] = root
-       }
+// The supplied maps must not be modified after calling
+// SetServiceRoots.
+func (kc *KeepClient) SetServiceRoots(locals, writables, gateways map[string]string) {
+       kc.disableDiscovery = true
+       kc.setServiceRoots(locals, writables, gateways)
+}
 
+func (kc *KeepClient) setServiceRoots(locals, writables, gateways map[string]string) {
        kc.lock.Lock()
        defer kc.lock.Unlock()
-       kc.localRoots = &locals
-       kc.writableLocalRoots = &writables
-       kc.gatewayRoots = &gateways
+       kc.localRoots = locals
+       kc.writableLocalRoots = writables
+       kc.gatewayRoots = gateways
 }
 
 // getSortedRoots returns a list of base URIs of Keep services, in the
index d2b84e94259892bcba2efe07e1952126530c8f7c..724d7ff3214db2315053d5e94f09e5e91adb070e 100644 (file)
@@ -35,6 +35,10 @@ type ServerRequiredSuite struct{}
 // Standalone tests
 type StandaloneSuite struct{}
 
+func (s *StandaloneSuite) SetUpTest(c *C) {
+       RefreshServiceDiscovery()
+}
+
 func pythonDir() string {
        cwd, _ := os.Getwd()
        return fmt.Sprintf("%s/../../python/tests", cwd)
@@ -50,6 +54,10 @@ func (s *ServerRequiredSuite) TearDownSuite(c *C) {
        arvadostest.StopAPI()
 }
 
+func (s *ServerRequiredSuite) SetUpTest(c *C) {
+       RefreshServiceDiscovery()
+}
+
 func (s *ServerRequiredSuite) TestMakeKeepClient(c *C) {
        arv, err := arvadosclient.MakeArvadosClient()
        c.Assert(err, Equals, nil)
@@ -1067,12 +1075,14 @@ func (s *StandaloneSuite) TestGetIndexWithNoPrefix(c *C) {
        defer ks.listener.Close()
 
        arv, err := arvadosclient.MakeArvadosClient()
-       kc, _ := MakeKeepClient(arv)
+       c.Assert(err, IsNil)
+       kc, err := MakeKeepClient(arv)
+       c.Assert(err, IsNil)
        arv.ApiToken = "abc123"
        kc.SetServiceRoots(map[string]string{"x": ks.url}, nil, nil)
 
        r, err := kc.GetIndex("x", "")
-       c.Check(err, Equals, nil)
+       c.Check(err, IsNil)
 
        content, err2 := ioutil.ReadAll(r)
        c.Check(err2, Equals, nil)
@@ -1098,7 +1108,7 @@ func (s *StandaloneSuite) TestGetIndexWithPrefix(c *C) {
        kc.SetServiceRoots(map[string]string{"x": ks.url}, nil, nil)
 
        r, err := kc.GetIndex("x", hash[0:3])
-       c.Check(err, Equals, nil)
+       c.Assert(err, Equals, nil)
 
        content, err2 := ioutil.ReadAll(r)
        c.Check(err2, Equals, nil)
@@ -1237,6 +1247,7 @@ func (s *ServerRequiredSuite) TestMakeKeepClientWithNonDiskTypeService(c *C) {
                &blobKeepService)
        c.Assert(err, Equals, nil)
        defer func() { arv.Delete("keep_services", blobKeepService["uuid"].(string), nil, nil) }()
+       RefreshServiceDiscovery()
 
        // Make a keepclient and ensure that the testblobstore is included
        kc, err := MakeKeepClient(arv)
index 0e74892a40951af958ba19137930d4ca63121b0a..8545cb80b855d9e53606042bfc940df7388e51eb 100644 (file)
@@ -10,9 +10,9 @@ import (
        "math/rand"
        "net/http"
        "os"
-       "regexp"
        "strings"
 
+       "git.curoverse.com/arvados.git/sdk/go/arvadosclient"
        "git.curoverse.com/arvados.git/sdk/go/streamer"
 )
 
@@ -22,8 +22,7 @@ import (
 var DebugPrintf = func(string, ...interface{}) {}
 
 func init() {
-       var matchTrue = regexp.MustCompile("^(?i:1|yes|true)$")
-       if matchTrue.MatchString(os.Getenv("ARVADOS_DEBUG")) {
+       if arvadosclient.StringBool(os.Getenv("ARVADOS_DEBUG")) {
                DebugPrintf = log.Printf
        }
 }
index 939ca212579c25c6e7f245fed6bf6e251fe4998a..73a7e09c5e69cdd6447d91f1e9d0907c97d143ce 100644 (file)
@@ -12,8 +12,8 @@ class Arvados::V1::CollectionsController < ApplicationController
   end
 
   def find_objects_for_index
-    if params[:include_trash] || ['destroy', 'trash'].include?(action_name)
-      @objects = Collection.unscoped.readable_by(*@read_users)
+    if params[:include_trash] || ['destroy', 'trash', 'untrash'].include?(action_name)
+      @objects = Collection.readable_by(*@read_users).unscoped
     end
     super
   end
@@ -63,6 +63,15 @@ class Arvados::V1::CollectionsController < ApplicationController
     show
   end
 
+  def untrash
+    if @object.is_trashed
+      @object.update_attributes!(trash_at: nil)
+    else
+      raise InvalidStateTransitionError
+    end
+    show
+  end
+
   def find_collections(visited, sp, &b)
     case sp
     when ArvadosModel
index d2a512bde75f68c790360813b47872b748481be1..e59c5f25789e4e83a9be0a215a784df6742d8eb2 100644 (file)
@@ -2,6 +2,7 @@ class Arvados::V1::KeepServicesController < ApplicationController
 
   skip_before_filter :find_object_by_uuid, only: :accessible
   skip_before_filter :render_404_if_no_object, only: :accessible
+  skip_before_filter :require_auth_scope, only: :accessible
 
   def find_objects_for_index
     # all users can list all keep services
index 77e5372a15423686d3597095c455d90513779bc9..87c4d91757a9daf9d086b56bf6ffcb0f6066cc71 100644 (file)
@@ -21,6 +21,7 @@ Server::Application.routes.draw do
         get 'provenance', on: :member
         get 'used_by', on: :member
         post 'trash', on: :member
+        post 'untrash', on: :member
       end
       resources :groups do
         get 'contents', on: :collection
index f48fbf1b8542d0f35fd37888c778fd264e820e7a..8aedbdc7057bc1d74d0749477a6a9c3c62b9624b 100644 (file)
@@ -297,7 +297,7 @@ unique_expired_collection:
   trash_at: 2001-01-01T00:00:00Z
   delete_at: 2038-01-01T00:00:00Z
   manifest_text: ". 29d7797f1888013986899bc9083783fa+3 0:3:expired\n"
-  name: unique_expired_collection
+  name: unique_expired_collection1
 
 unique_expired_collection2:
   uuid: zzzzz-4zz18-mto52zx1s7sn3jr
index a31ad8af03b09fd54dd80360ad8eefa65328e298..761898560a7caa60cf1c9610ea02851493240393 100644 (file)
@@ -1024,4 +1024,22 @@ EOS
       assert_operator c.delete_at, :>=, time_before_trashing + Rails.configuration.default_trash_lifetime
     end
   end
+
+  test 'untrash a trashed collection' do
+    authorize_with :active
+    post :untrash, {
+      id: collections(:expired_collection).uuid,
+    }
+    assert_response 200
+    assert_equal false, json_response['is_trashed']
+    assert_nil json_response['trash_at']
+  end
+
+  test 'untrash error on not trashed collection' do
+    authorize_with :active
+    post :untrash, {
+      id: collections(:collection_owned_by_active).uuid,
+    }
+    assert_response 422
+  end
 end
index 1375d4c9ce71549bdb43988cf4808eb70a908bf2..706f73ffda6157812e9cb687c9340f344464c663 100644 (file)
@@ -20,9 +20,9 @@ class Arvados::V1::KeepServicesControllerTest < ActionController::TestCase
     assert_equal true, assigns(:objects).any?
   end
 
-  [:admin, :active, :inactive, :anonymous].each do |u|
-    test "accessible to #{u} user" do
-      authorize_with u
+  [:admin, :active, :inactive, :anonymous, nil].each do |u|
+    test "accessible to #{u.inspect} user" do
+      authorize_with(u) if u
       get :accessible
       assert_response :success
       assert_not_empty json_response['items']
index 85ec93b8d9b325f00029613902d3dbda9a125adf..b7e39c6041d256188350d464da86ee39fb29bdeb 100644 (file)
@@ -64,6 +64,7 @@ func parseCollectionIDFromURL(s string) string {
 
 func (h *handler) setup() {
        h.clientPool = arvadosclient.MakeClientPool()
+       keepclient.RefreshServiceDiscoveryOnSIGHUP()
 }
 
 // ServeHTTP implements http.Handler.
index f9239a0f15648b0078a8ea8a00fce3dfc27d9366..7dfd01ad41e7fd96f74b763d6129cc1792448538 100644 (file)
@@ -104,6 +104,7 @@ func main() {
        if err != nil {
                log.Fatalf("Error setting up keep client %s", err.Error())
        }
+       keepclient.RefreshServiceDiscoveryOnSIGHUP()
 
        if cfg.PIDFile != "" {
                f, err := os.Create(cfg.PIDFile)
@@ -133,7 +134,6 @@ func main() {
        if cfg.DefaultReplicas > 0 {
                kc.Want_replicas = cfg.DefaultReplicas
        }
-       go kc.RefreshServices(5*time.Minute, 3*time.Second)
 
        listener, err = net.Listen("tcp", cfg.Listen)
        if err != nil {
index c0a7c6f6a523725098596e5fce89fcbffa7fdc94..34c2f61a37a01097ec1ce257f49f5ccb9c33e246 100644 (file)
@@ -29,24 +29,23 @@ func SetupPullWorkerIntegrationTest(t *testing.T, testData PullWorkIntegrationTe
        // start api and keep servers
        arvadostest.StartAPI()
        arvadostest.StartKeep(2, false)
+       keepclient.RefreshServiceDiscovery()
 
        // make arvadosclient
        arv, err := arvadosclient.MakeArvadosClient()
        if err != nil {
-               t.Error("Error creating arv")
+               t.Fatalf("Error creating arv: %s", err)
        }
 
        // keep client
-       keepClient = &keepclient.KeepClient{
-               Arvados:       arv,
-               Want_replicas: 1,
+       keepClient, err = keepclient.MakeKeepClient(arv)
+       if err != nil {
+               t.Fatalf("error creating KeepClient: %s", err)
        }
+       keepClient.Want_replicas = 1
 
        // discover keep services
        var servers []string
-       if err := keepClient.DiscoverKeepServers(); err != nil {
-               t.Error("Error discovering keep services")
-       }
        for _, host := range keepClient.LocalRoots() {
                servers = append(servers, host)
        }
index 6cf11a728075c60990c1b049c0fb916cea5cd5d5..e22e4b5cfe56e2d127d334ef593d6b5067cc3978 100644 (file)
@@ -5,15 +5,15 @@ import (
        "errors"
        "flag"
        "fmt"
-       "git.curoverse.com/arvados.git/sdk/go/arvadosclient"
-       "git.curoverse.com/arvados.git/sdk/go/keepclient"
        "io/ioutil"
        "log"
        "net/http"
        "os"
-       "regexp"
        "strings"
        "time"
+
+       "git.curoverse.com/arvados.git/sdk/go/arvadosclient"
+       "git.curoverse.com/arvados.git/sdk/go/keepclient"
 )
 
 func main() {
@@ -99,8 +99,6 @@ func loadConfig(configFile string) (config apiConfig, blobSigningKey string, err
        return
 }
 
-var matchTrue = regexp.MustCompile("^(?i:1|yes|true)$")
-
 // Read config from file
 func readConfigFromFile(filename string) (config apiConfig, blobSigningKey string, err error) {
        if !strings.Contains(filename, "/") {
@@ -130,9 +128,9 @@ func readConfigFromFile(filename string) (config apiConfig, blobSigningKey strin
                        case "ARVADOS_API_HOST":
                                config.APIHost = value
                        case "ARVADOS_API_HOST_INSECURE":
-                               config.APIHostInsecure = matchTrue.MatchString(value)
+                               config.APIHostInsecure = arvadosclient.StringBool(value)
                        case "ARVADOS_EXTERNAL_CLIENT":
-                               config.ExternalClient = matchTrue.MatchString(value)
+                               config.ExternalClient = arvadosclient.StringBool(value)
                        case "ARVADOS_BLOB_SIGNING_KEY":
                                blobSigningKey = value
                        }
@@ -153,7 +151,7 @@ func setupKeepClient(config apiConfig, keepServicesJSON string, blobSignatureTTL
                External: config.ExternalClient,
        }
 
-       // if keepServicesJSON is provided, use it to load services; else, use DiscoverKeepServers
+       // If keepServicesJSON is provided, use it instead of service discovery
        if keepServicesJSON == "" {
                kc, err = keepclient.MakeKeepClient(&arv)
                if err != nil {
index e49fe68616626275f00ba5bce21877dd969207dd..34d4f022bf8d7d4570e2465fd7d811dd3e1a2aa3 100644 (file)
@@ -12,6 +12,7 @@ import (
        "testing"
        "time"
 
+       "git.curoverse.com/arvados.git/sdk/go/arvadosclient"
        "git.curoverse.com/arvados.git/sdk/go/arvadostest"
        "git.curoverse.com/arvados.git/sdk/go/keepclient"
 
@@ -64,6 +65,7 @@ func (s *DoMainTestSuite) SetUpSuite(c *C) {
 func (s *DoMainTestSuite) SetUpTest(c *C) {
        logOutput := io.MultiWriter(&logBuffer)
        log.SetOutput(logOutput)
+       keepclient.RefreshServiceDiscovery()
 }
 
 func (s *DoMainTestSuite) TearDownTest(c *C) {
@@ -79,7 +81,7 @@ func setupKeepBlockCheckWithTTL(c *C, enforcePermissions bool, keepServicesJSON
        var config apiConfig
        config.APIHost = os.Getenv("ARVADOS_API_HOST")
        config.APIToken = arvadostest.DataManagerToken
-       config.APIHostInsecure = matchTrue.MatchString(os.Getenv("ARVADOS_API_HOST_INSECURE"))
+       config.APIHostInsecure = arvadosclient.StringBool(os.Getenv("ARVADOS_API_HOST_INSECURE"))
 
        // Start Keep servers
        arvadostest.StartKeep(2, enforcePermissions)
@@ -89,6 +91,8 @@ func setupKeepBlockCheckWithTTL(c *C, enforcePermissions bool, keepServicesJSON
        kc, ttl, err = setupKeepClient(config, keepServicesJSON, ttl)
        c.Assert(ttl, Equals, blobSignatureTTL)
        c.Check(err, IsNil)
+
+       keepclient.RefreshServiceDiscovery()
 }
 
 // Setup test data
@@ -144,9 +148,8 @@ func setupBlockHashFile(c *C, name string, blocks []string) string {
 
 func checkErrorLog(c *C, blocks []string, prefix, suffix string) {
        for _, hash := range blocks {
-               expected := prefix + `.*` + hash + `.*` + suffix
-               match, _ := regexp.MatchString(expected, logBuffer.String())
-               c.Assert(match, Equals, true)
+               expected := `(?ms).*` + prefix + `.*` + hash + `.*` + suffix + `.*`
+               c.Check(logBuffer.String(), Matches, expected)
        }
 }
 
@@ -288,7 +291,7 @@ func (s *ServerRequiredSuite) TestLoadConfig(c *C) {
 
        c.Assert(config.APIHost, Equals, os.Getenv("ARVADOS_API_HOST"))
        c.Assert(config.APIToken, Equals, arvadostest.DataManagerToken)
-       c.Assert(config.APIHostInsecure, Equals, matchTrue.MatchString(os.Getenv("ARVADOS_API_HOST_INSECURE")))
+       c.Assert(config.APIHostInsecure, Equals, arvadosclient.StringBool(os.Getenv("ARVADOS_API_HOST_INSECURE")))
        c.Assert(config.ExternalClient, Equals, false)
        c.Assert(blobSigningKey, Equals, "abcdefg")
 }
index c6e7665caa2a312c327b8a603159a7da07941450..b1513a02a7e30f3211b39bbf0a4335c8dd9cf8d9 100644 (file)
@@ -6,15 +6,15 @@ import (
        "errors"
        "flag"
        "fmt"
-       "git.curoverse.com/arvados.git/sdk/go/arvadosclient"
-       "git.curoverse.com/arvados.git/sdk/go/keepclient"
        "io/ioutil"
        "log"
        "net/http"
        "os"
-       "regexp"
        "strings"
        "time"
+
+       "git.curoverse.com/arvados.git/sdk/go/arvadosclient"
+       "git.curoverse.com/arvados.git/sdk/go/keepclient"
 )
 
 func main() {
@@ -119,8 +119,6 @@ func loadConfig(configFile string) (config apiConfig, blobSigningKey string, err
        return
 }
 
-var matchTrue = regexp.MustCompile("^(?i:1|yes|true)$")
-
 // Read config from file
 func readConfigFromFile(filename string) (config apiConfig, blobSigningKey string, err error) {
        if !strings.Contains(filename, "/") {
@@ -149,9 +147,9 @@ func readConfigFromFile(filename string) (config apiConfig, blobSigningKey strin
                case "ARVADOS_API_HOST":
                        config.APIHost = value
                case "ARVADOS_API_HOST_INSECURE":
-                       config.APIHostInsecure = matchTrue.MatchString(value)
+                       config.APIHostInsecure = arvadosclient.StringBool(value)
                case "ARVADOS_EXTERNAL_CLIENT":
-                       config.ExternalClient = matchTrue.MatchString(value)
+                       config.ExternalClient = arvadosclient.StringBool(value)
                case "ARVADOS_BLOB_SIGNING_KEY":
                        blobSigningKey = value
                }
@@ -170,7 +168,7 @@ func setupKeepClient(config apiConfig, keepServicesJSON string, isDst bool, repl
                External: config.ExternalClient,
        }
 
-       // if keepServicesJSON is provided, use it to load services; else, use DiscoverKeepServers
+       // If keepServicesJSON is provided, use it instead of service discovery
        if keepServicesJSON == "" {
                kc, err = keepclient.MakeKeepClient(&arv)
                if err != nil {
index 09609eb7498bb8dc28d95bc41892f4cca9ec8563..fec1f354c957e1ca5f106f666c632af3728d3b82 100644 (file)
@@ -4,35 +4,42 @@ import (
        "crypto/md5"
        "fmt"
        "io/ioutil"
-       "log"
        "os"
        "strings"
        "testing"
        "time"
 
+       "git.curoverse.com/arvados.git/sdk/go/arvadosclient"
        "git.curoverse.com/arvados.git/sdk/go/arvadostest"
        "git.curoverse.com/arvados.git/sdk/go/keepclient"
 
        . "gopkg.in/check.v1"
 )
 
+var kcSrc, kcDst *keepclient.KeepClient
+var srcKeepServicesJSON, dstKeepServicesJSON, blobSigningKey string
+var blobSignatureTTL = time.Duration(2*7*24) * time.Hour
+
+func resetGlobals() {
+       blobSigningKey = ""
+       srcKeepServicesJSON = ""
+       dstKeepServicesJSON = ""
+       kcSrc = nil
+       kcDst = nil
+}
+
 // Gocheck boilerplate
 func Test(t *testing.T) {
        TestingT(t)
 }
 
-// Gocheck boilerplate
 var _ = Suite(&ServerRequiredSuite{})
 var _ = Suite(&ServerNotRequiredSuite{})
 var _ = Suite(&DoMainTestSuite{})
 
-// Tests that require the Keep server running
 type ServerRequiredSuite struct{}
-type ServerNotRequiredSuite struct{}
-type DoMainTestSuite struct{}
 
 func (s *ServerRequiredSuite) SetUpSuite(c *C) {
-       // Start API server
        arvadostest.StartAPI()
 }
 
@@ -41,36 +48,32 @@ func (s *ServerRequiredSuite) TearDownSuite(c *C) {
        arvadostest.ResetEnv()
 }
 
-var initialArgs []string
-
-func (s *DoMainTestSuite) SetUpSuite(c *C) {
-       initialArgs = os.Args
-}
-
-var kcSrc, kcDst *keepclient.KeepClient
-var srcKeepServicesJSON, dstKeepServicesJSON, blobSigningKey string
-var blobSignatureTTL = time.Duration(2*7*24) * time.Hour
-
 func (s *ServerRequiredSuite) SetUpTest(c *C) {
-       // reset all variables between tests
-       blobSigningKey = ""
-       srcKeepServicesJSON = ""
-       dstKeepServicesJSON = ""
-       kcSrc = &keepclient.KeepClient{}
-       kcDst = &keepclient.KeepClient{}
+       resetGlobals()
 }
 
 func (s *ServerRequiredSuite) TearDownTest(c *C) {
        arvadostest.StopKeep(3)
 }
 
+func (s *ServerNotRequiredSuite) SetUpTest(c *C) {
+       resetGlobals()
+}
+
+type ServerNotRequiredSuite struct{}
+
+type DoMainTestSuite struct {
+       initialArgs []string
+}
+
 func (s *DoMainTestSuite) SetUpTest(c *C) {
-       args := []string{"keep-rsync"}
-       os.Args = args
+       s.initialArgs = os.Args
+       os.Args = []string{"keep-rsync"}
+       resetGlobals()
 }
 
 func (s *DoMainTestSuite) TearDownTest(c *C) {
-       os.Args = initialArgs
+       os.Args = s.initialArgs
 }
 
 var testKeepServicesJSON = "{ \"kind\":\"arvados#keepServiceList\", \"etag\":\"\", \"self_link\":\"\", \"offset\":null, \"limit\":null, \"items\":[ { \"href\":\"/keep_services/zzzzz-bi6l4-123456789012340\", \"kind\":\"arvados#keepService\", \"etag\":\"641234567890enhj7hzx432e5\", \"uuid\":\"zzzzz-bi6l4-123456789012340\", \"owner_uuid\":\"zzzzz-tpzed-123456789012345\", \"service_host\":\"keep0.zzzzz.arvadosapi.com\", \"service_port\":25107, \"service_ssl_flag\":false, \"service_type\":\"disk\", \"read_only\":false }, { \"href\":\"/keep_services/zzzzz-bi6l4-123456789012341\", \"kind\":\"arvados#keepService\", \"etag\":\"641234567890enhj7hzx432e5\", \"uuid\":\"zzzzz-bi6l4-123456789012341\", \"owner_uuid\":\"zzzzz-tpzed-123456789012345\", \"service_host\":\"keep0.zzzzz.arvadosapi.com\", \"service_port\":25108, \"service_ssl_flag\":false, \"service_type\":\"disk\", \"read_only\":false } ], \"items_available\":2 }"
@@ -83,13 +86,13 @@ func setupRsync(c *C, enforcePermissions bool, replications int) {
        var srcConfig apiConfig
        srcConfig.APIHost = os.Getenv("ARVADOS_API_HOST")
        srcConfig.APIToken = arvadostest.DataManagerToken
-       srcConfig.APIHostInsecure = matchTrue.MatchString(os.Getenv("ARVADOS_API_HOST_INSECURE"))
+       srcConfig.APIHostInsecure = arvadosclient.StringBool(os.Getenv("ARVADOS_API_HOST_INSECURE"))
 
        // dstConfig
        var dstConfig apiConfig
        dstConfig.APIHost = os.Getenv("ARVADOS_API_HOST")
        dstConfig.APIToken = arvadostest.DataManagerToken
-       dstConfig.APIHostInsecure = matchTrue.MatchString(os.Getenv("ARVADOS_API_HOST_INSECURE"))
+       dstConfig.APIHostInsecure = arvadosclient.StringBool(os.Getenv("ARVADOS_API_HOST_INSECURE"))
 
        if enforcePermissions {
                blobSigningKey = arvadostest.BlobSigningKey
@@ -97,45 +100,30 @@ func setupRsync(c *C, enforcePermissions bool, replications int) {
 
        // Start Keep servers
        arvadostest.StartKeep(3, enforcePermissions)
+       keepclient.RefreshServiceDiscovery()
 
        // setup keepclients
        var err error
        kcSrc, _, err = setupKeepClient(srcConfig, srcKeepServicesJSON, false, 0, blobSignatureTTL)
-       c.Check(err, IsNil)
+       c.Assert(err, IsNil)
 
        kcDst, _, err = setupKeepClient(dstConfig, dstKeepServicesJSON, true, replications, 0)
-       c.Check(err, IsNil)
+       c.Assert(err, IsNil)
 
-       for uuid := range kcSrc.LocalRoots() {
+       srcRoots := map[string]string{}
+       dstRoots := map[string]string{}
+       for uuid, root := range kcSrc.LocalRoots() {
                if strings.HasSuffix(uuid, "02") {
-                       delete(kcSrc.LocalRoots(), uuid)
+                       dstRoots[uuid] = root
+               } else {
+                       srcRoots[uuid] = root
                }
        }
-       for uuid := range kcSrc.GatewayRoots() {
-               if strings.HasSuffix(uuid, "02") {
-                       delete(kcSrc.GatewayRoots(), uuid)
-               }
+       if srcKeepServicesJSON == "" {
+               kcSrc.SetServiceRoots(srcRoots, srcRoots, srcRoots)
        }
-       for uuid := range kcSrc.WritableLocalRoots() {
-               if strings.HasSuffix(uuid, "02") {
-                       delete(kcSrc.WritableLocalRoots(), uuid)
-               }
-       }
-
-       for uuid := range kcDst.LocalRoots() {
-               if strings.HasSuffix(uuid, "00") || strings.HasSuffix(uuid, "01") {
-                       delete(kcDst.LocalRoots(), uuid)
-               }
-       }
-       for uuid := range kcDst.GatewayRoots() {
-               if strings.HasSuffix(uuid, "00") || strings.HasSuffix(uuid, "01") {
-                       delete(kcDst.GatewayRoots(), uuid)
-               }
-       }
-       for uuid := range kcDst.WritableLocalRoots() {
-               if strings.HasSuffix(uuid, "00") || strings.HasSuffix(uuid, "01") {
-                       delete(kcDst.WritableLocalRoots(), uuid)
-               }
+       if dstKeepServicesJSON == "" {
+               kcDst.SetServiceRoots(dstRoots, dstRoots, dstRoots)
        }
 
        if replications == 0 {
@@ -188,22 +176,8 @@ func (s *ServerRequiredSuite) TestRsyncInitializeWithKeepServicesJSON(c *C) {
 
        localRoots := kcSrc.LocalRoots()
        c.Check(localRoots, NotNil)
-
-       foundIt := false
-       for k := range localRoots {
-               if k == "zzzzz-bi6l4-123456789012340" {
-                       foundIt = true
-               }
-       }
-       c.Check(foundIt, Equals, true)
-
-       foundIt = false
-       for k := range localRoots {
-               if k == "zzzzz-bi6l4-123456789012341" {
-                       foundIt = true
-               }
-       }
-       c.Check(foundIt, Equals, true)
+       c.Check(localRoots["zzzzz-bi6l4-123456789012340"], Not(Equals), "")
+       c.Check(localRoots["zzzzz-bi6l4-123456789012341"], Not(Equals), "")
 }
 
 // Test keep-rsync initialization with default replications count
@@ -329,8 +303,8 @@ func (s *ServerRequiredSuite) TestErrorDuringRsync_FakeSrcKeepservers(c *C) {
        setupRsync(c, false, 1)
 
        err := performKeepRsync(kcSrc, kcDst, blobSignatureTTL, "", "")
-       log.Printf("Err = %v", err)
-       c.Check(strings.Contains(err.Error(), "no such host"), Equals, true)
+       c.Assert(err, NotNil)
+       c.Check(err.Error(), Matches, ".*no such host.*")
 }
 
 // Setup rsync using dstKeepServicesJSON with fake keepservers.
@@ -341,8 +315,8 @@ func (s *ServerRequiredSuite) TestErrorDuringRsync_FakeDstKeepservers(c *C) {
        setupRsync(c, false, 1)
 
        err := performKeepRsync(kcSrc, kcDst, blobSignatureTTL, "", "")
-       log.Printf("Err = %v", err)
-       c.Check(strings.Contains(err.Error(), "no such host"), Equals, true)
+       c.Assert(err, NotNil)
+       c.Check(err.Error(), Matches, ".*no such host.*")
 }
 
 // Test rsync with signature error during Get from src.
@@ -356,7 +330,8 @@ func (s *ServerRequiredSuite) TestErrorDuringRsync_ErrorGettingBlockFromSrc(c *C
        blobSigningKey = "thisisfakeblobsigningkey"
 
        err := performKeepRsync(kcSrc, kcDst, blobSignatureTTL, blobSigningKey, "")
-       c.Check(strings.Contains(err.Error(), "HTTP 403 \"Forbidden\""), Equals, true)
+       c.Assert(err, NotNil)
+       c.Check(err.Error(), Matches, ".*HTTP 403 \"Forbidden\".*")
 }
 
 // Test rsync with error during Put to src.
@@ -370,7 +345,8 @@ func (s *ServerRequiredSuite) TestErrorDuringRsync_ErrorPuttingBlockInDst(c *C)
        kcDst.Want_replicas = 2
 
        err := performKeepRsync(kcSrc, kcDst, blobSignatureTTL, blobSigningKey, "")
-       c.Check(strings.Contains(err.Error(), "Could not write sufficient replicas"), Equals, true)
+       c.Assert(err, NotNil)
+       c.Check(err.Error(), Matches, ".*Could not write sufficient replicas.*")
 }
 
 // Test loadConfig func
@@ -391,7 +367,7 @@ func (s *ServerNotRequiredSuite) TestLoadConfig(c *C) {
 
        c.Assert(srcConfig.APIHost, Equals, os.Getenv("ARVADOS_API_HOST"))
        c.Assert(srcConfig.APIToken, Equals, arvadostest.DataManagerToken)
-       c.Assert(srcConfig.APIHostInsecure, Equals, matchTrue.MatchString(os.Getenv("ARVADOS_API_HOST_INSECURE")))
+       c.Assert(srcConfig.APIHostInsecure, Equals, arvadosclient.StringBool(os.Getenv("ARVADOS_API_HOST_INSECURE")))
        c.Assert(srcConfig.ExternalClient, Equals, false)
 
        dstConfig, _, err := loadConfig(dstConfigFile)
@@ -399,7 +375,7 @@ func (s *ServerNotRequiredSuite) TestLoadConfig(c *C) {
 
        c.Assert(dstConfig.APIHost, Equals, os.Getenv("ARVADOS_API_HOST"))
        c.Assert(dstConfig.APIToken, Equals, arvadostest.DataManagerToken)
-       c.Assert(dstConfig.APIHostInsecure, Equals, matchTrue.MatchString(os.Getenv("ARVADOS_API_HOST_INSECURE")))
+       c.Assert(dstConfig.APIHostInsecure, Equals, arvadosclient.StringBool(os.Getenv("ARVADOS_API_HOST_INSECURE")))
        c.Assert(dstConfig.ExternalClient, Equals, false)
 
        c.Assert(srcBlobSigningKey, Equals, "abcdefg")
@@ -414,15 +390,15 @@ func (s *ServerNotRequiredSuite) TestLoadConfig_MissingSrcConfig(c *C) {
 // Test loadConfig func - error reading config
 func (s *ServerNotRequiredSuite) TestLoadConfig_ErrorLoadingSrcConfig(c *C) {
        _, _, err := loadConfig("no-such-config-file")
-       c.Assert(strings.Contains(err.Error(), "no such file or directory"), Equals, true)
+       c.Assert(err, NotNil)
+       c.Check(err.Error(), Matches, ".*no such file or directory.*")
 }
 
 func (s *ServerNotRequiredSuite) TestSetupKeepClient_NoBlobSignatureTTL(c *C) {
        var srcConfig apiConfig
        srcConfig.APIHost = os.Getenv("ARVADOS_API_HOST")
        srcConfig.APIToken = arvadostest.DataManagerToken
-       srcConfig.APIHostInsecure = matchTrue.MatchString(os.Getenv("ARVADOS_API_HOST_INSECURE"))
-       arvadostest.StartKeep(2, false)
+       srcConfig.APIHostInsecure = arvadosclient.StringBool(os.Getenv("ARVADOS_API_HOST_INSECURE"))
 
        _, ttl, err := setupKeepClient(srcConfig, srcKeepServicesJSON, false, 0, 0)
        c.Check(err, IsNil)
@@ -448,7 +424,7 @@ func setupConfigFile(c *C, name string) *os.File {
 
 func (s *DoMainTestSuite) Test_doMain_NoSrcConfig(c *C) {
        err := doMain()
-       c.Check(err, NotNil)
+       c.Assert(err, NotNil)
        c.Assert(err.Error(), Equals, "Error loading src configuration from file: config file not specified")
 }
 
@@ -457,7 +433,7 @@ func (s *DoMainTestSuite) Test_doMain_SrcButNoDstConfig(c *C) {
        args := []string{"-replications", "3", "-src", srcConfig.Name()}
        os.Args = append(os.Args, args...)
        err := doMain()
-       c.Check(err, NotNil)
+       c.Assert(err, NotNil)
        c.Assert(err.Error(), Equals, "Error loading dst configuration from file: config file not specified")
 }
 
@@ -465,8 +441,8 @@ func (s *DoMainTestSuite) Test_doMain_BadSrcConfig(c *C) {
        args := []string{"-src", "abcd"}
        os.Args = append(os.Args, args...)
        err := doMain()
-       c.Check(err, NotNil)
-       c.Assert(strings.HasPrefix(err.Error(), "Error loading src configuration from file: Error reading config file"), Equals, true)
+       c.Assert(err, NotNil)
+       c.Assert(err.Error(), Matches, "Error loading src configuration from file: Error reading config file.*")
 }
 
 func (s *DoMainTestSuite) Test_doMain_WithReplicationsButNoSrcConfig(c *C) {
@@ -488,6 +464,7 @@ func (s *DoMainTestSuite) Test_doMainWithSrcAndDstConfig(c *C) {
        // actual copying to dst will happen, but that's ok.
        arvadostest.StartKeep(2, false)
        defer arvadostest.StopKeep(2)
+       keepclient.RefreshServiceDiscovery()
 
        err := doMain()
        c.Check(err, IsNil)