Skip to content

Commit

Permalink
Merge pull request #29 from alexander-demichev/apifix
Browse files Browse the repository at this point in the history
✨ Remove unnecessary pointers from API
  • Loading branch information
k8s-ci-robot authored Feb 21, 2022
2 parents d4243f6 + e409a59 commit 0f17404
Show file tree
Hide file tree
Showing 12 changed files with 38 additions and 114 deletions.
23 changes: 8 additions & 15 deletions api/v1alpha1/provider_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ type ProviderSpec struct {
// The contents should be in the form of key:value. This secret must be in
// the same namespace as the provider.
// +optional
SecretName *string `json:"secretName,omitempty"`
SecretName string `json:"secretName,omitempty"`

// FetchConfig determines how the operator will fetch the components and metadata for the provider.
// If nil, the operator will try to fetch components according to default
Expand All @@ -70,14 +70,13 @@ type ManagerSpec struct {
// Default empty, meaning the profiler is disabled.
// Controller Manager flag is --profiler-address.
// +optional
ProfilerAddress *string `json:"profilerAddress,omitempty"`
ProfilerAddress string `json:"profilerAddress,omitempty"`

// MaxConcurrentReconciles is the maximum number of concurrent Reconciles
// which can be run. Defaults to 10.
// which can be run.
// +optional
// +kubebuilder:default=10
// +kubebuilder:validation:Minimum=1
MaxConcurrentReconciles *int `json:"maxConcurrentReconciles,omitempty"`
MaxConcurrentReconciles int `json:"maxConcurrentReconciles,omitempty"`

// Verbosity set the logs verbosity. Defaults to 1.
// Controller Manager flag is --verbosity.
Expand All @@ -86,12 +85,6 @@ type ManagerSpec struct {
// +kubebuilder:validation:Minimum=0
Verbosity int `json:"verbosity,omitempty"`

// Debug, if set, will override a set of fields with opinionated values for
// a debugging session. (Verbosity=5, ProfilerAddress=localhost:6060)
// +optional
// +kubebuilder:default=false
Debug bool `json:"debug,omitempty"`

// FeatureGates define provider specific feature flags that will be passed
// in as container args to the provider's controller manager.
// Controller Manager flag is --feature-gates.
Expand Down Expand Up @@ -160,15 +153,15 @@ type ContainerSpec struct {
type ImageMeta struct {
// Repository sets the container registry to pull images from.
// +optional
Repository *string `json:"repository,omitempty"`
Repository string `json:"repository,omitempty"`

// Name allows to specify a name for the image.
// +optional
Name *string `json:"name,omitempty"`
Name string `json:"name,omitempty"`

// Tag allows to specify a tag for the image.
// +optional
Tag *string `json:"tag,omitempty"`
Tag string `json:"tag,omitempty"`
}

// FetchConfiguration determines the way to fetch the components and metadata for the provider.
Expand All @@ -178,7 +171,7 @@ type FetchConfiguration struct {
// You must set `providerSpec.Version` field for operator to pick up
// desired version of the release from GitHub.
// +optional
URL *string `json:"url,omitempty"`
URL string `json:"url,omitempty"`

// Selector to be used for fetching provider’s components and metadata from
// ConfigMaps stored inside the cluster. Each ConfigMap is expected to contain
Expand Down
37 changes: 1 addition & 36 deletions api/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -1254,11 +1254,6 @@ spec:
of version) would be `ReplicaSet.apps`."
type: object
type: object
debug:
default: false
description: Debug, if set, will override a set of fields with
opinionated values for a debugging session. (Verbosity=5, ProfilerAddress=localhost:6060)
type: boolean
featureGates:
additionalProperties:
type: boolean
Expand Down Expand Up @@ -1341,9 +1336,8 @@ spec:
- retryPeriod
type: object
maxConcurrentReconciles:
default: 10
description: MaxConcurrentReconciles is the maximum number of
concurrent Reconciles which can be run. Defaults to 10.
concurrent Reconciles which can be run.
minimum: 1
type: integer
metrics:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1255,11 +1255,6 @@ spec:
of version) would be `ReplicaSet.apps`."
type: object
type: object
debug:
default: false
description: Debug, if set, will override a set of fields with
opinionated values for a debugging session. (Verbosity=5, ProfilerAddress=localhost:6060)
type: boolean
featureGates:
additionalProperties:
type: boolean
Expand Down Expand Up @@ -1342,9 +1337,8 @@ spec:
- retryPeriod
type: object
maxConcurrentReconciles:
default: 10
description: MaxConcurrentReconciles is the maximum number of
concurrent Reconciles which can be run. Defaults to 10.
concurrent Reconciles which can be run.
minimum: 1
type: integer
metrics:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1254,11 +1254,6 @@ spec:
of version) would be `ReplicaSet.apps`."
type: object
type: object
debug:
default: false
description: Debug, if set, will override a set of fields with
opinionated values for a debugging session. (Verbosity=5, ProfilerAddress=localhost:6060)
type: boolean
featureGates:
additionalProperties:
type: boolean
Expand Down Expand Up @@ -1341,9 +1336,8 @@ spec:
- retryPeriod
type: object
maxConcurrentReconciles:
default: 10
description: MaxConcurrentReconciles is the maximum number of
concurrent Reconciles which can be run. Defaults to 10.
concurrent Reconciles which can be run.
minimum: 1
type: integer
metrics:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1255,11 +1255,6 @@ spec:
of version) would be `ReplicaSet.apps`."
type: object
type: object
debug:
default: false
description: Debug, if set, will override a set of fields with
opinionated values for a debugging session. (Verbosity=5, ProfilerAddress=localhost:6060)
type: boolean
featureGates:
additionalProperties:
type: boolean
Expand Down Expand Up @@ -1342,9 +1337,8 @@ spec:
- retryPeriod
type: object
maxConcurrentReconciles:
default: 10
description: MaxConcurrentReconciles is the maximum number of
concurrent Reconciles which can be run. Defaults to 10.
concurrent Reconciles which can be run.
minimum: 1
type: integer
metrics:
Expand Down
24 changes: 8 additions & 16 deletions controllers/component_customizer.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package controllers

import (
"fmt"
"math"
"sort"
"strings"
"time"
Expand Down Expand Up @@ -127,8 +126,8 @@ func customizeManager(mSpec *operatorv1.ManagerSpec, c *corev1.Container) {
c.Args = setArgs(c.Args, "--"+strings.ToLower(k)+"-concurrency", fmt.Sprint(v))
}
}
if mSpec.MaxConcurrentReconciles != nil {
c.Args = setArgs(c.Args, "--max-concurrent-reconciles", fmt.Sprint(*mSpec.MaxConcurrentReconciles))
if mSpec.MaxConcurrentReconciles != 0 {
c.Args = setArgs(c.Args, "--max-concurrent-reconciles", fmt.Sprint(mSpec.MaxConcurrentReconciles))
}

if mSpec.CacheNamespace != "" {
Expand Down Expand Up @@ -179,21 +178,14 @@ func customizeManager(mSpec *operatorv1.ManagerSpec, c *corev1.Container) {
}
}

if mSpec.ProfilerAddress != nil {
c.Args = setArgs(c.Args, "--profiler-address", *mSpec.ProfilerAddress)
if mSpec.ProfilerAddress != "" {
c.Args = setArgs(c.Args, "--profiler-address", mSpec.ProfilerAddress)
}

if mSpec.Verbosity != defaultVerbosity {
c.Args = setArgs(c.Args, "--v", fmt.Sprint(mSpec.Verbosity))
}

if mSpec.Debug {
c.Args = setArgs(c.Args, "--v", fmt.Sprint(math.Max(5, float64(mSpec.Verbosity))))
if mSpec.ProfilerAddress == nil { // don't override ProfilerAddress if set.
c.Args = setArgs(c.Args, "--profiler-address", "localhost:6060")
}
}

if len(mSpec.FeatureGates) > 0 {
fgValue := []string{}
for fg, val := range mSpec.FeatureGates {
Expand Down Expand Up @@ -223,7 +215,7 @@ func customizeContainer(cSpec operatorv1.ContainerSpec, d *appsv1.Deployment) {
if cSpec.Resources != nil {
c.Resources = *cSpec.Resources
}
if cSpec.Image != nil && cSpec.Image.Name != nil && cSpec.Image.Repository != nil {
if cSpec.Image != nil && cSpec.Image.Name != "" && cSpec.Image.Repository != "" {
c.Image = imageMetaToURL(cSpec.Image)
}
if cSpec.Command != nil {
Expand Down Expand Up @@ -261,10 +253,10 @@ func removeEnv(envs []corev1.EnvVar, name string) []corev1.EnvVar {
// imageMetaToURL translate container image meta to URL.
func imageMetaToURL(im *operatorv1.ImageMeta) string {
tag := "latest"
if im.Tag != nil {
tag = *im.Tag
if im.Tag != "" {
tag = im.Tag
}
return strings.Join([]string{*im.Repository, *im.Name}, "/") + ":" + tag
return strings.Join([]string{im.Repository, im.Name}, "/") + ":" + tag
}

// leaderElectionArgs set leader election flags.
Expand Down
16 changes: 8 additions & 8 deletions controllers/component_customizer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,9 +203,9 @@ func TestCustomizeDeployment(t *testing.T) {
{
Name: "manager",
Image: &operatorv1.ImageMeta{
Name: pointer.StringPtr("mydns"),
Repository: pointer.StringPtr("quay.io/dev"),
Tag: pointer.StringPtr("v3.4.2"),
Name: "mydns",
Repository: "quay.io/dev",
Tag: "v3.4.2",
},
Env: []corev1.EnvVar{
{
Expand Down Expand Up @@ -302,9 +302,9 @@ func TestCustomizeDeployment(t *testing.T) {
{
Name: "manager",
Image: &operatorv1.ImageMeta{
Name: pointer.StringPtr("mydns"),
Repository: pointer.StringPtr("quay.io/dev"),
Tag: pointer.StringPtr("v3.4.2"),
Name: "mydns",
Repository: "quay.io/dev",
Tag: "v3.4.2",
},
Env: []corev1.EnvVar{
{
Expand Down Expand Up @@ -414,9 +414,9 @@ func TestCustomizeDeployment(t *testing.T) {
{
name: "all manager options",
inputManagerSpec: &operatorv1.ManagerSpec{
Debug: true,
FeatureGates: map[string]bool{"TEST": true, "ANOTHER": false},
ProfilerAddress: pointer.StringPtr("localhost:1234"),
ProfilerAddress: "localhost:1234",
Verbosity: 5,
ControllerManagerConfigurationSpec: configv1.ControllerManagerConfigurationSpec{
CacheNamespace: "testNS",
SyncPeriod: &metav1.Duration{Duration: sevenHours},
Expand Down
8 changes: 4 additions & 4 deletions controllers/phases.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,9 +172,9 @@ func (p *phaseReconciler) secretReader(ctx context.Context) (configclient.Reader
}

// Fetch configutation variables from the secret. See API field docs for more info.
if p.provider.GetSpec().SecretName != nil {
if p.provider.GetSpec().SecretName != "" {
secret := &corev1.Secret{}
key := types.NamespacedName{Namespace: p.provider.GetNamespace(), Name: *p.provider.GetSpec().SecretName}
key := types.NamespacedName{Namespace: p.provider.GetNamespace(), Name: p.provider.GetSpec().SecretName}
if err := p.ctrlClient.Get(ctx, key, secret); err != nil {
return nil, err
}
Expand All @@ -186,9 +186,9 @@ func (p *phaseReconciler) secretReader(ctx context.Context) (configclient.Reader
}

// If provided store fetch config url in memory reader.
if p.provider.GetSpec().FetchConfig != nil && p.provider.GetSpec().FetchConfig.URL != nil {
if p.provider.GetSpec().FetchConfig != nil && p.provider.GetSpec().FetchConfig.URL != "" {
log.V(2).Info("Custom fetch configuration url was provided", "provider", p.provider.GetName())
return mr.AddProvider(p.provider.GetName(), util.ClusterctlProviderType(p.provider), *p.provider.GetSpec().FetchConfig.URL)
return mr.AddProvider(p.provider.GetName(), util.ClusterctlProviderType(p.provider), p.provider.GetSpec().FetchConfig.URL)
}

return mr, nil
Expand Down
5 changes: 2 additions & 3 deletions controllers/phases_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
. "github.com/onsi/gomega"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/utils/pointer"
operatorv1 "sigs.k8s.io/cluster-api-operator/api/v1alpha1"
"sigs.k8s.io/cluster-api-operator/controllers/genericprovider"
"sigs.k8s.io/cluster-api/cmd/clusterctl/client/repository"
Expand Down Expand Up @@ -52,9 +51,9 @@ func TestSecretReader(t *testing.T) {
},
Spec: operatorv1.CoreProviderSpec{
ProviderSpec: operatorv1.ProviderSpec{
SecretName: &secretName,
SecretName: secretName,
FetchConfig: &operatorv1.FetchConfiguration{
URL: pointer.StringPtr("https://example.com"),
URL: "https://example.com",
},
},
},
Expand Down
2 changes: 1 addition & 1 deletion controllers/preflight_checks.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func preflightChecks(ctx context.Context, c client.Client, provider genericprovi
return ctrl.Result{RequeueAfter: preflightFailedRequeueAfter}, fmt.Errorf("version contains invalid value for provider %s", provider.GetName())
}

if spec.FetchConfig != nil && spec.FetchConfig.Selector != nil && spec.FetchConfig.URL != nil {
if spec.FetchConfig != nil && spec.FetchConfig.Selector != nil && spec.FetchConfig.URL != "" {
// If FetchConfiguration is not nil, exactly one of `URL` or `Selector` must be specified.
conditions.Set(provider, conditions.FalseCondition(
operatorv1.PreflightCheckCondition,
Expand Down
Loading

0 comments on commit 0f17404

Please sign in to comment.