Skip to content

Commit

Permalink
Merge pull request #3132 from spadgett/metrics-warning
Browse files Browse the repository at this point in the history 
Bug 1721428: Remove misleading "Metrics might not be available" warning
  • Loading branch information
@openshift-merge-robot
openshift-merge-robot authored Jun 18, 2019
2 parents a820396 + b5bdb7b commit c8657b9
Show file tree
Hide file tree
Showing 8 changed files with 51 additions and 147 deletions.
5 changes: 0 additions & 5 deletions app/scripts/controllers/create/createFromImage.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,11 +148,6 @@ angular.module("openshiftConsole")
return scope.buildConfig.sourceUrl === _.get(scope, 'image.metadata.annotations.sampleRepo');
};

// Warn if metrics aren't configured when setting autoscaling options.
MetricsService.isAvailable().then(function(available) {
$scope.metricsWarning = !available;
});

var configMapDataOrdered = [];
var secretDataOrdered = [];
$scope.valueFromObjects = [];
Expand Down
5 changes: 0 additions & 5 deletions app/scripts/controllers/edit/autoscaler.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,6 @@ angular.module('openshiftConsole')

$scope.labels = [];

// Warn if metrics aren't configured when setting autoscaling options.
MetricsService.isAvailable().then(function(available) {
$scope.metricsWarning = !available;
});

var getErrorDetails = $filter('getErrorDetails');

var navigateBack = function() {
Expand Down
32 changes: 9 additions & 23 deletions app/scripts/services/hpa.js
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
'use strict';

angular.module("openshiftConsole")
.factory("HPAService", function($filter, $q, LimitRangesService, MetricsService) {
.factory("HPAService", function($filter, $q, LimitRangesService) {

var annotation = $filter('annotation');

Expand Down Expand Up @@ -77,17 +77,6 @@ angular.module("openshiftConsole")
var hasDeploymentConfig = $filter('hasDeploymentConfig');


var hasMetricsAvailableWarning = function(metricsAvailable) {
if (!metricsAvailable) {
return {
message: 'Metrics might not be configured by your cluster administrator. ' +
'Metrics are required for autoscaling.',
reason: 'MetricsNotAvailable'
};
}
};


var hasCPURequestWarning = function(scaleTarget, limitRanges, project) {
var containers = _.get(scaleTarget, 'spec.template.spec.containers', []);
var kind;
Expand Down Expand Up @@ -155,17 +144,14 @@ angular.module("openshiftConsole")
if (!scaleTarget || _.isEmpty(hpaResources)) {
return $q.when([]);
}
return MetricsService.isAvailable().then(function(metricsAvailable) {
var isV2HPA = hasV2HPAAnnotations(hpaResources);
return _.compact([
hasMetricsAvailableWarning(metricsAvailable),
// we don't need to show this warning, but if we have it, we should also
// not show the cpuRequestWarning, until we update to the newer API
isV2HPA ? false : hasCPURequestWarning(scaleTarget, limitRanges, project),
hasCompetingAutoscalersWarning(hpaResources),
hasCompetingDCAndAutoscalerWarning(scaleTarget, hpaResources)
]);
});
var isV2HPA = hasV2HPAAnnotations(hpaResources);
return $q.when(_.compact([
// we don't need to show this warning, but if we have it, we should also
// not show the cpuRequestWarning, until we update to the newer API
isV2HPA ? false : hasCPURequestWarning(scaleTarget, limitRanges, project),
hasCompetingAutoscalersWarning(hpaResources),
hasCompetingDCAndAutoscalerWarning(scaleTarget, hpaResources)
]));
};


Expand Down
6 changes: 0 additions & 6 deletions app/views/create/fromimage.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -313,12 +313,6 @@ <h3>Environment Variables <span class="appended-icon">(Runtime only) <span class
<div class="learn-more-block">
<a href="{{'pod_autoscaling' | helpLink}}" target="_blank">Learn More&nbsp;<i class="fa fa-external-link" aria-hidden="true"></i></a>
</div>
<div class="has-warning" ng-if="metricsWarning && scaling.autoscale">
<span class="help-block">
CPU metrics might not be available. In order to use horizontal pod autoscalers,
your cluster administrator must have properly configured cluster metrics.
</span>
</div>
</div>
<div ng-if="!scaling.autoscale"
class="form-group"
Expand Down
10 changes: 1 addition & 9 deletions app/views/edit/autoscaler.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,10 @@ <h1>
<div class="help-block">
Scale replicas automatically based on CPU usage.
</div>
<div class="learn-more-block" ng-class="{ 'gutter-bottom': metricsWarning || showCPURequestWarning }">
<div class="learn-more-block" ng-class="{ 'gutter-bottom': showCPURequestWarning }">
<a href="{{'pod_autoscaling' | helpLink}}" target="_blank">Learn More&nbsp;<i class="fa fa-external-link" aria-hidden> </i></a>
</div>

<!-- Warning: metrics not configured -->
<div ng-if="metricsWarning" class="alert alert-warning">
<span class="pficon pficon-warning-triangle-o" aria-hidden="true"></span>
<span class="sr-only">Warning:</span>
Metrics might not be configured by your cluster administrator. Metrics are
required for autoscaling.
</div>

<!-- Warning: CPU request not set -->
<div ng-if="showCPURequestWarning" class="alert alert-warning">
<span class="pficon pficon-warning-triangle-o" aria-hidden="true"></span>
Expand Down
90 changes: 40 additions & 50 deletions dist/scripts/scripts.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3270,45 +3270,40 @@ controller: "SetHomePageModalController"
});
}
};
} ]), angular.module("openshiftConsole").factory("HPAService", [ "$filter", "$q", "LimitRangesService", "MetricsService", function(e, t, n, r) {
var a = e("annotation"), o = function(e, t, n) {
} ]), angular.module("openshiftConsole").factory("HPAService", [ "$filter", "$q", "LimitRangesService", function(e, t, n) {
var r = e("annotation"), a = function(e, t, n) {
return _.every(n, function(n) {
return _.get(n, [ "resources", t, e ]);
});
}, o = function(e, t) {
return a(e, "requests", t);
}, i = function(e, t) {
return o(e, "requests", t);
}, s = function(e, t) {
return o(e, "limits", t);
}, c = function(e, t, r) {
return a(e, "limits", t);
}, s = function(e, t, r) {
return !!n.getEffectiveLimitRange(r, e, "Container")[t];
}, c = function(e, t) {
return s(e, "defaultRequest", t);
}, l = function(e, t) {
return c(e, "defaultRequest", t);
}, u = function(e, t) {
return c(e, "defaultLimit", t);
}, d = function(e, t, r) {
return !!n.hasClusterResourceOverrides(r) || (!(!i("cpu", e) && !l("cpu", t)) || !(!s("cpu", e) && !u("cpu", t)));
}, m = e("humanizeKind"), p = e("hasDeploymentConfig"), f = function(e) {
if (!e) return {
message: "Metrics might not be configured by your cluster administrator. Metrics are required for autoscaling.",
reason: "MetricsNotAvailable"
};
}, g = function(e, t, n) {
return s(e, "defaultLimit", t);
}, u = function(e, t, r) {
return !!n.hasClusterResourceOverrides(r) || (!(!o("cpu", e) && !c("cpu", t)) || !(!i("cpu", e) && !l("cpu", t)));
}, d = e("humanizeKind"), m = e("hasDeploymentConfig"), p = function(e, t, n) {
var r, a = _.get(e, "spec.template.spec.containers", []);
if (!d(a, t, n)) return r = m(e.kind), {
if (!u(a, t, n)) return r = d(e.kind), {
message: "This " + r + " does not have any containers with a CPU request set. Autoscaling will not work without a CPU request.",
reason: "NoCPURequest"
};
}, v = function(e) {
}, f = function(e) {
return _.some(e, function(e) {
return a(e, "autoscaling.alpha.kubernetes.io/metrics");
return r(e, "autoscaling.alpha.kubernetes.io/metrics");
});
}, h = function(e) {
}, g = function(e) {
if (_.size(e) > 1) return {
message: "More than one autoscaler is scaling this resource. This is not recommended because they might compete with each other. Consider removing all but one autoscaler.",
reason: "MultipleHPA"
};
}, y = function(e, t) {
if ("ReplicationController" === e.kind && p(e) && _.some(t, function() {
}, v = function(e, t) {
if ("ReplicationController" === e.kind && m(e) && _.some(t, function() {
return _.some(t, function(e) {
return "ReplicationController" === _.get(e, "spec.scaleTargetRef.kind");
});
Expand All @@ -3319,19 +3314,18 @@ reason: "DeploymentHasHPA"
};
return {
usesV2Metrics: function(e) {
return v([ e ]);
return f([ e ]);
},
hasCPURequest: d,
hasCPURequest: u,
filterHPA: function(e, t, n) {
return _.filter(e, function(e) {
return e.spec.scaleTargetRef.kind === t && e.spec.scaleTargetRef.name === n;
});
},
getHPAWarnings: function(e, n, a, o) {
return !e || _.isEmpty(n) ? t.when([]) : r.isAvailable().then(function(t) {
var r = v(n);
return _.compact([ f(t), !r && g(e, a, o), h(n), y(e, n) ]);
});
getHPAWarnings: function(e, n, r, a) {
if (!e || _.isEmpty(n)) return t.when([]);
var o = f(n);
return t.when(_.compact([ !o && p(e, r, a), g(n), v(e, n) ]));
},
groupHPAs: function(e) {
var t = {};
Expand Down Expand Up @@ -7807,9 +7801,7 @@ var f = [ "Deployment", "DeploymentConfig", "HorizontalPodAutoscaler", "ReplicaS
if (_.includes(f, n.kind)) {
e.kind = n.kind, e.name = n.name, "HorizontalPodAutoscaler" === n.kind ? e.disableInputs = !0 : (e.targetKind = n.kind, e.targetName = n.name), e.autoscaling = {
name: e.name
}, e.labels = [], l.isAvailable().then(function(t) {
e.metricsWarning = !t;
});
}, e.labels = [];
var g = t("getErrorDetails"), v = function() {
r.history.back();
};
Expand Down Expand Up @@ -8192,14 +8184,14 @@ e.cpuProblems = d.validatePodLimits(e.limitRanges, "cpu", [ e.container ], t), e
c.list(A, n).then(function(t) {
e.limitRanges = t.by("metadata.name"), _.isEmpty(e.limitRanges) || e.$watch("container", i, !0);
});
var v, y, C = function() {
var p, v, y = function() {
e.scaling.autoscale && !e.hasClusterResourceOverrides ? e.showCPURequestWarning = !l.hasCPURequest([ e.container ], e.limitRanges, t) : e.showCPURequestWarning = !1;
};
c.list(L, n).then(function(e) {
v = e.by("metadata.name"), m.log("quotas", v);
p = e.by("metadata.name"), m.log("quotas", p);
}), c.list(O, n).then(function(e) {
y = e.by("metadata.name"), m.log("cluster quotas", y);
}), e.$watch("scaling.autoscale", C), e.$watch("container", C, !0), e.$watch("name", function(e, t) {
v = e.by("metadata.name"), m.log("cluster quotas", v);
}), e.$watch("scaling.autoscale", y), e.$watch("container", y, !0), e.$watch("name", function(e, t) {
I.value && I.value !== t || (I.value = e);
}), function(r) {
r.name = a.name, r.imageName = k, r.imageTag = a.imageTag, r.namespace = a.namespace, r.buildConfig = {
Expand Down Expand Up @@ -8237,9 +8229,7 @@ var e;
(r.image || r.image.metadata || r.image.metadata.annotations) && (e = r.image.metadata.annotations, r.buildConfig.sourceUrl = e.sampleRepo || "", r.buildConfig.gitRef = e.sampleRef || "", r.buildConfig.contextDir = e.sampleContextDir || "", (e.sampleRef || e.sampleContextDir) && (r.advancedSourceOptions = !0));
}, r.usingSampleRepo = function() {
return r.buildConfig.sourceUrl === _.get(r, "image.metadata.annotations.sampleRepo");
}, p.isAvailable().then(function(t) {
e.metricsWarning = !t;
});
};
var o = [], i = [];
e.valueFromObjects = [], c.list(D, n, null, {
errorNotification: !1
Expand Down Expand Up @@ -8293,15 +8283,15 @@ f.toErrorPage("Cannot create from source: the specified image could not be retri
f.toErrorPage("Cannot create from source: the specified image could not be retrieved.");
});
}(e);
var U, F = function() {
var C, U = function() {
var t = {
started: "Creating application " + e.name + " in project " + e.projectDisplayName(),
success: "Created application " + e.name + " in project " + e.projectDisplayName(),
failure: "Failed to create " + e.name + " in project " + e.projectDisplayName()
}, o = {};
S.clear(), S.add(t, o, a.project, function() {
var t = r.defer();
return c.batch(U, n).then(function(n) {
return c.batch(C, n).then(function(n) {
var r = [], a = !1;
_.isEmpty(n.failure) ? r.push({
type: "success",
Expand All @@ -8325,7 +8315,7 @@ hasErrors: a
}), f.toNextSteps(e.name, e.projectName, {
usingSampleRepo: e.usingSampleRepo()
});
}, x = function(e) {
}, F = function(e) {
o.open({
templateUrl: "views/modals/confirm.html",
controller: "ConfirmModalController",
Expand All @@ -8341,26 +8331,26 @@ cancelButtonText: "Cancel"
};
}
}
}).result.then(F);
}, M = function(t) {
}).result.then(U);
}, x = function(t) {
N(), T = t.quotaAlerts || [], e.nameTaken || _.some(T, {
type: "error"
}) ? (e.disableInputs = !1, _.each(T, function(e) {
e.id = _.uniqueId("create-builder-alert-"), g.addNotification(e);
})) : _.isEmpty(T) ? F() : (x(T), e.disableInputs = !1);
})) : _.isEmpty(T) ? U() : (F(T), e.disableInputs = !1);
};
e.projectDisplayName = function() {
return P(this.project) || this.projectName;
}, e.createApp = function() {
e.disableInputs = !0, N(), e.buildConfig.envVars = w.compactEntries(e.buildConfigEnvVars), e.deploymentConfig.envVars = w.compactEntries(e.DCEnvVarsFromUser), e.labels = w.mapEntries(w.compactEntries(e.labelArray));
var t = s.generate(e);
U = [], angular.forEach(t, function(e) {
null !== e && (m.debug("Generated resource definition:", e), U.push(e));
C = [], angular.forEach(t, function(e) {
null !== e && (m.debug("Generated resource definition:", e), C.push(e));
});
var r = s.ifResourcesDontExist(U, e.projectName), a = h.getLatestQuotaAlerts(U, n), o = function(t) {
var r = s.ifResourcesDontExist(C, e.projectName), a = h.getLatestQuotaAlerts(C, n), o = function(t) {
return e.nameTaken = t.nameTaken, a;
};
r.then(o, o).then(M, M);
r.then(o, o).then(x, x);
};
})), e.cancel = function() {
f.toProjectOverview(e.projectName);
Expand Down
13 changes: 1 addition & 12 deletions dist/scripts/templates.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5024,11 +5024,6 @@ angular.module('openshiftConsoleTemplates', []).run(['$templateCache', function(
"<div class=\"learn-more-block\">\n" +
"<a href=\"{{'pod_autoscaling' | helpLink}}\" target=\"_blank\">Learn More&nbsp;<i class=\"fa fa-external-link\" aria-hidden=\"true\"></i></a>\n" +
"</div>\n" +
"<div class=\"has-warning\" ng-if=\"metricsWarning && scaling.autoscale\">\n" +
"<span class=\"help-block\">\n" +
"CPU metrics might not be available. In order to use horizontal pod autoscalers, your cluster administrator must have properly configured cluster metrics.\n" +
"</span>\n" +
"</div>\n" +
"</div>\n" +
"<div ng-if=\"!scaling.autoscale\" class=\"form-group\" ng-class=\"{ 'has-error': form.replicas.$dirty && form.replicas.$invalid }\">\n" +
"<label class=\"number\">Replicas</label>\n" +
Expand Down Expand Up @@ -9502,16 +9497,10 @@ angular.module('openshiftConsoleTemplates', []).run(['$templateCache', function(
"<div class=\"help-block\">\n" +
"Scale replicas automatically based on CPU usage.\n" +
"</div>\n" +
"<div class=\"learn-more-block\" ng-class=\"{ 'gutter-bottom': metricsWarning || showCPURequestWarning }\">\n" +
"<div class=\"learn-more-block\" ng-class=\"{ 'gutter-bottom': showCPURequestWarning }\">\n" +
"<a href=\"{{'pod_autoscaling' | helpLink}}\" target=\"_blank\">Learn More&nbsp;<i class=\"fa fa-external-link\" aria-hidden> </i></a>\n" +
"</div>\n" +
"\n" +
"<div ng-if=\"metricsWarning\" class=\"alert alert-warning\">\n" +
"<span class=\"pficon pficon-warning-triangle-o\" aria-hidden=\"true\"></span>\n" +
"<span class=\"sr-only\">Warning:</span>\n" +
"Metrics might not be configured by your cluster administrator. Metrics are required for autoscaling.\n" +
"</div>\n" +
"\n" +
"<div ng-if=\"showCPURequestWarning\" class=\"alert alert-warning\">\n" +
"<span class=\"pficon pficon-warning-triangle-o\" aria-hidden=\"true\"></span>\n" +
"<span class=\"sr-only\">Warning:</span>\n" +
Expand Down
37 changes: 0 additions & 37 deletions test/spec/services/hpaServiceSpec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,43 +93,6 @@ describe('HPAService', function() {
});
});

describe('When metrics are not configured', function() {
it('should return a warning with reason MetricsNotAvailable', function() {
// flip the bool that controls the MetricsService mock
metricsAreAvailable = false;
HPAService
.getHPAWarnings(defaultScaleTarget, defaultHpaResources, defaultLimitRanges, defaultProject)
.then(function(warnings) {
expect(_.head(warnings).reason).toEqual('MetricsNotAvailable');
});
$rootScope.$digest();
});
});


// TODO: we should be testing more than just the happy paths
// describe('When there is a CPU request', function() {
// // this is the first check in the hasCPURequest fn
// // it should use $window && set OPENSHIFT_CONFIG.clusterResourceOverridesEnabled
// // to trigger the LimitRangesService.hasClusterResourceOverrides function
// // it('should not return a warning when cluster resource overrides are enabled', function() {
// //
// // });
//
// // 2nd check in the hasCPURequest fn
// // need to test this with some mock limit ranges passed to the fn
// // it('should not return a warning when containers or limit ranges have a cpu request', function() {
// //
// // });
//
// // 3rd check in the hasCPURequest fn
// // more tests with mock limit ranges
// // it('should not return a warning when the containers or limit ranges have a cpu limit', function() {
// //
// // });
//
// });

describe('When the scale target does not have a CPU request set', function() {
var scaleTargetWithoutLimit = {
kind: '',
Expand Down

0 comments on commit c8657b9

Please sign in to comment.