From daa43c3bb959b5a8d637c85d90f17e63a64f14e4 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Mon, 18 Aug 2014 20:34:39 -0400 Subject: [PATCH 1/3] Add better messaging around pulling of base images when they fail due to invalid or missing credentials --- data/buildlogs.py | 7 +++- static/css/quay.css | 4 +- static/directives/build-log-error.html | 27 ++++++++++++-- static/js/app.js | 51 +++++++++++++++++++++++++- static/partials/repo-build.html | 2 +- workers/dockerfilebuild.py | 14 ++++++- 6 files changed, 93 insertions(+), 12 deletions(-) diff --git a/data/buildlogs.py b/data/buildlogs.py index 2ccd03899..8f184de27 100644 --- a/data/buildlogs.py +++ b/data/buildlogs.py @@ -25,7 +25,7 @@ class RedisBuildLogs(object): """ return self._redis.rpush(self._logs_key(build_id), json.dumps(log_obj)) - def append_log_message(self, build_id, log_message, log_type=None): + def append_log_message(self, build_id, log_message, log_type=None, log_data=None): """ Wraps the message in an envelope and push it to the end of the log entry list and returns the index at which it was inserted. @@ -37,6 +37,9 @@ class RedisBuildLogs(object): if log_type: log_obj['type'] = log_type + if log_data: + log_obj['data'] = log_data + return self._redis.rpush(self._logs_key(build_id), json.dumps(log_obj)) - 1 def get_log_entries(self, build_id, start_index): @@ -106,4 +109,4 @@ class BuildLogs(object): return buildlogs def __getattr__(self, name): - return getattr(self.state, name, None) \ No newline at end of file + return getattr(self.state, name, None) diff --git a/static/css/quay.css b/static/css/quay.css index 01fe84e60..68834f282 100644 --- a/static/css/quay.css +++ b/static/css/quay.css @@ -2535,7 +2535,7 @@ p.editable:hover i { margin-top: 10px; } -.repo-build .build-log-error-element { +.repo-build .build-log-error-element .error-message-container { position: relative; display: inline-block; margin: 10px; @@ -2545,7 +2545,7 @@ p.editable:hover i { margin-left: 22px; } -.repo-build .build-log-error-element i.fa { +.repo-build .build-log-error-element .error-message-container i.fa { color: red; position: absolute; top: 13px; diff --git a/static/directives/build-log-error.html b/static/directives/build-log-error.html index 095f8edd0..cf03fa7b2 100644 --- a/static/directives/build-log-error.html +++ b/static/directives/build-log-error.html @@ -1,4 +1,23 @@ - - - - +
+ + + + + caused by attempting to pull private repository {{ getLocalPullInfo().repo }} + with inaccessible crdentials + without credentials + + + + +
+
+ Note: The credentials {{ getLocalPullInfo().login.username }} for registry {{ getLocalPullInfo().login.registry }} cannot + access repository {{ getLocalPullInfo().repo }}. +
+
+ Note: No robot account is specified for this build. Without such credentials, this pull will always fail. Please setup a new + build trigger with a robot account that has access to {{ getLocalPullInfo().repo }} or make the repository public. +
+
+
diff --git a/static/js/app.js b/static/js/app.js index ad6527fc1..00c1a27ce 100644 --- a/static/js/app.js +++ b/static/js/app.js @@ -113,6 +113,14 @@ quayApp = angular.module('quay', quayDependencies, function($provide, cfpLoading this.currentIndex_ = 0; } + _ViewArray.prototype.length = function() { + return this.entries.length; + }; + + _ViewArray.prototype.get = function(index) { + return this.entries[index]; + }; + _ViewArray.prototype.push = function(elem) { this.entries.push(elem); this.hasEntries = true; @@ -4021,9 +4029,48 @@ quayApp.directive('buildLogError', function () { transclude: false, restrict: 'C', scope: { - 'error': '=error' + 'error': '=error', + 'entries': '=entries' }, - controller: function($scope, $element) { + controller: function($scope, $element, Config) { + $scope.getLocalPullInfo = function() { + if ($scope.entries.__localpull !== undefined) { + return $scope.entries.__localpull; + } + + var localInfo = { + 'isLocal': false + }; + + // Find the 'pulling' phase entry, and then extra any metadata found under + // it. + for (var i = 0; i < $scope.entries.length; ++i) { + var entry = $scope.entries[i]; + if (entry.type == 'phase' && entry.message == 'pulling') { + for (var j = 0; j < entry.logs.length(); ++j) { + var log = entry.logs.get(j); + if (log.data && log.data.phasestep == 'login') { + localInfo['login'] = log.data; + } + + if (log.data && log.data.phasestep == 'pull') { + var repo_url = log.data['repo_url']; + var repo_and_tag = repo_url.substring(Config.SERVER_HOSTNAME.length + 1); + var tagIndex = repo_and_tag.lastIndexOf(':'); + var repo = repo_and_tag.substring(0, tagIndex); + + localInfo['repo_url'] = repo_url; + localInfo['repo'] = repo; + + localInfo['isLocal'] = repo_url.indexOf(Config.SERVER_HOSTNAME + '/') == 0; + } + } + break; + } + } + + return $scope.entries.__localpull = localInfo; + }; } }; return directiveDefinitionObject; diff --git a/static/partials/repo-build.html b/static/partials/repo-build.html index 3afe87508..225f58701 100644 --- a/static/partials/repo-build.html +++ b/static/partials/repo-build.html @@ -77,7 +77,7 @@
- +
diff --git a/workers/dockerfilebuild.py b/workers/dockerfilebuild.py index a4de1cc47..d200a336e 100644 --- a/workers/dockerfilebuild.py +++ b/workers/dockerfilebuild.py @@ -223,6 +223,13 @@ class DockerfileBuildContext(object): if self._pull_credentials: logger.debug('Logging in with pull credentials: %s@%s', self._pull_credentials['username'], self._pull_credentials['registry']) + + self._build_logger('Pulling base image: %s' % image_and_tag, { + 'phasestep': 'login', + 'username': self._pull_credentials['username'], + 'registry': self._pull_credentials['registry'] + }) + self._build_cl.login(self._pull_credentials['username'], self._pull_credentials['password'], registry=self._pull_credentials['registry'], reauth=True) @@ -233,7 +240,12 @@ class DockerfileBuildContext(object): raise JobException('Missing FROM command in Dockerfile') image_and_tag = ':'.join(image_and_tag_tuple) - self._build_logger('Pulling base image: %s' % image_and_tag) + + self._build_logger('Pulling base image: %s' % image_and_tag, { + 'phasestep': 'pull', + 'repo_url': image_and_tag + }) + pull_status = self._build_cl.pull(image_and_tag, stream=True) self.__monitor_completion(pull_status, 'Downloading', self._status, 'pull_completion') From 07aab4274c1a54543e93bbf1a1c176fd002d9831 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Thu, 28 Aug 2014 19:19:20 -0400 Subject: [PATCH 2/3] Fix parameters for logging the extra data needed --- workers/dockerfilebuild.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/workers/dockerfilebuild.py b/workers/dockerfilebuild.py index d200a336e..8442237bd 100644 --- a/workers/dockerfilebuild.py +++ b/workers/dockerfilebuild.py @@ -224,7 +224,7 @@ class DockerfileBuildContext(object): logger.debug('Logging in with pull credentials: %s@%s', self._pull_credentials['username'], self._pull_credentials['registry']) - self._build_logger('Pulling base image: %s' % image_and_tag, { + self._build_logger('Pulling base image: %s' % image_and_tag, log_data = { 'phasestep': 'login', 'username': self._pull_credentials['username'], 'registry': self._pull_credentials['registry'] @@ -241,7 +241,7 @@ class DockerfileBuildContext(object): image_and_tag = ':'.join(image_and_tag_tuple) - self._build_logger('Pulling base image: %s' % image_and_tag, { + self._build_logger('Pulling base image: %s' % image_and_tag, log_data = { 'phasestep': 'pull', 'repo_url': image_and_tag }) From da3d58890e2df5dc5d7e8aa02ffe117f6ca989c8 Mon Sep 17 00:00:00 2001 From: Jake Moshenko Date: Fri, 12 Sep 2014 10:46:35 -0400 Subject: [PATCH 3/3] Slight tweak in the text of the 403 pull base image error. --- static/directives/build-log-error.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/static/directives/build-log-error.html b/static/directives/build-log-error.html index cf03fa7b2..13b399bb9 100644 --- a/static/directives/build-log-error.html +++ b/static/directives/build-log-error.html @@ -17,7 +17,7 @@
Note: No robot account is specified for this build. Without such credentials, this pull will always fail. Please setup a new - build trigger with a robot account that has access to {{ getLocalPullInfo().repo }} or make the repository public. + build trigger with a robot account that has access to {{ getLocalPullInfo().repo }} or make that repository public.