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

Bug 1809353: Exclude Kubernetes control plane rules when running on IBM Cloud #705

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions pkg/client/client.go
Expand Up @@ -239,6 +239,10 @@ func (c *Client) GetProxy(name string) (*configv1.Proxy, error) {
return c.oscclient.ConfigV1().Proxies().Get(name, metav1.GetOptions{})
}

func (c *Client) GetInfrastructure(name string) (*configv1.Infrastructure, error) {
return c.oscclient.ConfigV1().Infrastructures().Get(name, metav1.GetOptions{})
}

func (c *Client) GetConfigmap(namespace, name string) (*v1.ConfigMap, error) {
return c.kclient.CoreV1().ConfigMaps(namespace).Get(name, metav1.GetOptions{})
}
Expand Down
14 changes: 12 additions & 2 deletions pkg/manifests/config.go
Expand Up @@ -27,8 +27,9 @@ import (
)

type Config struct {
Images *Images `json:"-"`
RemoteWrite bool `json:"-"`
Images *Images `json:"-"`
RemoteWrite bool `json:"-"`
Platform configv1.PlatformType `json:"-"`

PrometheusOperatorConfig *PrometheusOperatorConfig `json:"prometheusOperator"`
PrometheusOperatorUserWorkloadConfig *PrometheusOperatorConfig `json:"prometheusOperatorUserWorkload"`
Expand Down Expand Up @@ -345,6 +346,15 @@ func (c *Config) LoadProxy(load func() (*configv1.Proxy, error)) error {
return nil
}

func (c *Config) LoadPlatform(load func() (*configv1.Infrastructure, error)) error {
i, err := load()
if err != nil {
return fmt.Errorf("error loading platform: %v", err)
}
c.Platform = i.Status.Platform
return nil
}

func NewConfigFromString(content string) (*Config, error) {
if content == "" {
return NewDefaultConfig(), nil
Expand Down
20 changes: 20 additions & 0 deletions pkg/manifests/manifests.go
Expand Up @@ -30,6 +30,7 @@ import (
"strings"

monv1 "github.com/coreos/prometheus-operator/pkg/apis/monitoring/v1"
configv1 "github.com/openshift/api/config/v1"
routev1 "github.com/openshift/api/route/v1"
securityv1 "github.com/openshift/api/security/v1"
"github.com/openshift/cluster-monitoring-operator/pkg/promqlgen"
Expand Down Expand Up @@ -219,6 +220,10 @@ var (
PrometheusTrustedCABundlePath = PrometheusTrustedCABundleDir + "ca-bundle.crt"
)

const (
IBMCloudPlatformType configv1.PlatformType = "IBMCloud"
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't redefine constant as vars, that's horribly dangerous.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thx! fixed

)

func MustAssetReader(asset string) io.Reader {
return bytes.NewReader(MustAsset(asset))
}
Expand Down Expand Up @@ -804,6 +809,21 @@ func (f *Factory) PrometheusK8sRules() (*monv1.PrometheusRule, error) {
r.Spec.Groups = groups
}

if f.config.Platform == IBMCloudPlatformType {
groups := []monv1.RuleGroup{}
for _, g := range r.Spec.Groups {
switch g.Name {
case "kubernetes-system-apiserver",
"kubernetes-system-controller-manager",
"kubernetes-system-scheduler":
// skip
default:
groups = append(groups, g)
}
}
r.Spec.Groups = groups
}

return r, nil
}

Expand Down
59 changes: 59 additions & 0 deletions pkg/manifests/manifests_test.go
Expand Up @@ -22,6 +22,8 @@ import (
"testing"

monv1 "github.com/coreos/prometheus-operator/pkg/apis/monitoring/v1"
configv1 "github.com/openshift/api/config/v1"

v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -1176,6 +1178,63 @@ func TestOpenShiftStateMetrics(t *testing.T) {
}
}

func TestPrometheusK8sControlPlaneRulesFiltered(t *testing.T) {
tests := []struct {
name string
config *Config
verify func(bool, bool, bool)
}{
{
name: "default config",
config: func() *Config {
c := NewDefaultConfig()
c.Platform = configv1.AWSPlatformType
return c
}(),
verify: func(api, cm, sched bool) {
if !api || !cm || !sched {
t.Fatal("did not get all expected kubernetes control plane rules")
}
},
},
{
name: "hosted control plane",
config: func() *Config {
c := NewDefaultConfig()
c.Platform = IBMCloudPlatformType
return c
}(),
verify: func(api, cm, sched bool) {
if api || cm || sched {
t.Fatalf("kubernetes control plane rules found, none expected")
}
},
},
}

for _, tc := range tests {
f := NewFactory("openshift-monitoring", "openshift-user-workload-monitoring", tc.config)
r, err := f.PrometheusK8sRules()
if err != nil {
t.Fatal(err)
}
apiServerRulesFound := false
controllerManagerRulesFound := false
schedulerRulesFound := false
for _, g := range r.Spec.Groups {
switch g.Name {
case "kubernetes-system-apiserver":
Copy link
Contributor

@brancz brancz Mar 16, 2020

Choose a reason for hiding this comment

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

what about etcd? and what about the servicemonitors that are provisioned to monitor these in the first place, they will now show "down" targets?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Service monitors that are part of control plane operators are skipped via annotation: https://github.com/openshift/cluster-kube-apiserver-operator/blob/4de583dffa47c25adde871e7361386b0d55c674a/manifests/0000_90_kube-apiserver-operator_03_servicemonitor.yaml#L7
Etcd rules are disabled because the etcd-metric-client secret is not placed in the openshift-config namespace:

s, err := o.client.GetSecret("openshift-config", "etcd-metric-client")
if err != nil {
klog.Warningf("Error loading etcd client secrets for Prometheus. Proceeding with etcd disabled. Error: %v", err)
}

Copy link
Contributor

Choose a reason for hiding this comment

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

I see ok.

apiServerRulesFound = true
case "kubernetes-system-controller-manager":
controllerManagerRulesFound = true
case "kubernetes-system-scheduler":
schedulerRulesFound = true
}
}
tc.verify(apiServerRulesFound, controllerManagerRulesFound, schedulerRulesFound)
}
}

func TestPrometheusEtcdRulesFiltered(t *testing.T) {
enabled := false
c := NewDefaultConfig()
Expand Down
7 changes: 7 additions & 0 deletions pkg/operator/operator.go
Expand Up @@ -410,6 +410,13 @@ func (o *Operator) Config(key string) *manifests.Config {
klog.Warningf("Could not load proxy configuration from API. This is expected and message can be ignored when proxy configuration doesn't exist. Proceeding without it: %v", err)
}

err = c.LoadPlatform(func() (*configv1.Infrastructure, error) {
return o.client.GetInfrastructure("cluster")
})
if err != nil {
klog.Warningf("Could not load platform from infrastructure resource: %v. This may result in alerts that are not appropriate for the platform.", err)
}

cm, err := o.client.GetConfigmap("openshift-config", "etcd-metric-serving-ca")
if err != nil {
klog.Warningf("Error loading etcd CA certificates for Prometheus. Proceeding with etcd disabled. Error: %v", err)
Expand Down