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

apply crds on operator startup #122

Merged
merged 3 commits into from
Nov 1, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,5 @@ RUN CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build -v -a -ldflags '-extldflags "
FROM scratch
WORKDIR /
COPY --from=builder /go/src/github.com/Azure/aks-app-routing-operator/aks-app-routing-operator .
COPY --from=builder /go/src/github.com/Azure/aks-app-routing-operator/config/crd/bases ./crd
ENTRYPOINT ["/aks-app-routing-operator"]
14 changes: 14 additions & 0 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"errors"
"flag"
"fmt"
"os"
"strings"
"time"

Expand Down Expand Up @@ -42,6 +43,7 @@ func init() {
flag.StringVar(&Flags.OperatorDeployment, "operator-deployment", "app-routing-operator", "name of the operator's k8s deployment")
flag.StringVar(&Flags.ClusterUid, "cluster-uid", "", "unique identifier of the cluster the add-on belongs to")
flag.DurationVar(&Flags.DnsSyncInterval, "dns-sync-interval", defaultDnsSyncInterval, "interval at which to sync DNS records")
flag.StringVar(&Flags.CrdPath, "crd", "/crd", "location of the CRD manifests")
}

type DnsZoneConfig struct {
Expand All @@ -64,6 +66,7 @@ type Config struct {
OperatorDeployment string
ClusterUid string
DnsSyncInterval time.Duration
CrdPath string
}

func (c *Config) Validate() error {
Expand Down Expand Up @@ -106,6 +109,17 @@ func (c *Config) Validate() error {
c.DnsSyncInterval = defaultDnsSyncInterval
}

crdPathStat, err := os.Stat(c.CrdPath)
if os.IsNotExist(err) {
return fmt.Errorf("crd path %s does not exist", c.CrdPath)
}
if err != nil {
return fmt.Errorf("checking crd path %s: %s", c.CrdPath, err)
}
if !crdPathStat.IsDir() {
return fmt.Errorf("crd path %s is not a directory", c.CrdPath)
}

return nil
}

Expand Down
41 changes: 41 additions & 0 deletions pkg/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,19 @@ package config

import (
"errors"
"fmt"
"strings"
"testing"

"github.com/stretchr/testify/require"
)

const (
validCrdPath = "../../config/crd/bases/"
notADirectoryPath = "./config.go"
notAValidPath = "./does/not/exist"
)

var validateTestCases = []struct {
Name string
Conf *Config
Expand All @@ -29,6 +36,7 @@ var validateTestCases = []struct {
ConcurrencyWatchdogThres: 101,
ConcurrencyWatchdogVotes: 2,
ClusterUid: "cluster-uid",
CrdPath: validCrdPath,
},
},
{
Expand All @@ -43,7 +51,40 @@ var validateTestCases = []struct {
ConcurrencyWatchdogThres: 101,
ConcurrencyWatchdogVotes: 2,
ClusterUid: "test-cluster-uid",
CrdPath: validCrdPath,
},
},
{
Name: "nonexistent crd path",
Conf: &Config{
NS: "test-namespace",
Registry: "test-registry",
MSIClientID: "test-msi-client-id",
TenantID: "test-tenant-id",
Cloud: "test-cloud",
Location: "test-location",
ConcurrencyWatchdogThres: 101,
ConcurrencyWatchdogVotes: 2,
ClusterUid: "test-cluster-uid",
CrdPath: notAValidPath,
},
Error: fmt.Sprintf("crd path %s does not exist", notAValidPath),
},
{
Name: "non-directory crd path",
Conf: &Config{
NS: "test-namespace",
Registry: "test-registry",
MSIClientID: "test-msi-client-id",
TenantID: "test-tenant-id",
Cloud: "test-cloud",
Location: "test-location",
ConcurrencyWatchdogThres: 101,
ConcurrencyWatchdogVotes: 2,
ClusterUid: "test-cluster-uid",
CrdPath: notADirectoryPath,
},
Error: fmt.Sprintf("crd path %s is not a directory", notADirectoryPath),
},
{
Name: "missing-namespace",
Expand Down
26 changes: 21 additions & 5 deletions pkg/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,13 @@ import (
"context"
"net/http"

approutingv1alpha1 "github.com/Azure/aks-app-routing-operator/api/v1alpha1"
"github.com/go-logr/logr"
cfgv1alpha2 "github.com/openservicemesh/osm/pkg/apis/config/v1alpha2"
policyv1alpha1 "github.com/openservicemesh/osm/pkg/apis/policy/v1alpha1"
ubzap "go.uber.org/zap"
appsv1 "k8s.io/api/apps/v1"
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
Expand All @@ -21,6 +23,7 @@ import (
"k8s.io/client-go/rest"
"k8s.io/klog/v2"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/log/zap"
metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
secv1 "sigs.k8s.io/secrets-store-csi-driver/apis/v1"
Expand All @@ -33,7 +36,9 @@ import (
"github.com/Azure/aks-app-routing-operator/pkg/controller/osm"
)

var scheme = runtime.NewScheme()
var (
scheme = runtime.NewScheme()
)

func init() {
registerSchemes(scheme)
Expand All @@ -57,6 +62,8 @@ func registerSchemes(s *runtime.Scheme) {
utilruntime.Must(secv1.Install(s))
utilruntime.Must(cfgv1alpha2.AddToScheme(s))
utilruntime.Must(policyv1alpha1.AddToScheme(s))
utilruntime.Must(approutingv1alpha1.AddToScheme(s))
utilruntime.Must(apiextensionsv1.AddToScheme(s))
}

func NewManager(conf *config.Config) (ctrl.Manager, error) {
Expand Down Expand Up @@ -84,17 +91,26 @@ func NewManagerForRestConfig(conf *config.Config, rc *rest.Config) (ctrl.Manager

m.AddHealthzCheck("liveness", func(req *http.Request) error { return nil })

kcs, err := kubernetes.NewForConfig(rc) // need non-caching client since manager hasn't started yet
// create non-caching clients, non-caching for use before manager has started
kcs, err := kubernetes.NewForConfig(rc)
if err != nil {
return nil, err
}
cl, err := client.New(rc, client.Options{Scheme: scheme})
if err != nil {
return nil, err
}

log := m.GetLogger()
deploy, err := getSelfDeploy(kcs, conf, log)
setupLog := m.GetLogger().WithName("setup")
deploy, err := getSelfDeploy(kcs, conf, setupLog)
if err != nil {
return nil, err
}
log.V(2).Info("using namespace: " + conf.NS)
setupLog.V(2).Info("using namespace: " + conf.NS)

if err := loadCRDs(cl, conf, setupLog); err != nil {
return nil, err
jaiveerk marked this conversation as resolved.
Show resolved Hide resolved
}

if err := dns.NewExternalDns(m, conf, deploy); err != nil {
return nil, err
Expand Down
27 changes: 0 additions & 27 deletions pkg/controller/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,41 +8,20 @@ import (
"context"
"encoding/json"
"errors"
"os"
"strings"
"testing"

"github.com/Azure/aks-app-routing-operator/pkg/config"
"github.com/Azure/aks-app-routing-operator/pkg/controller/testutils"
"github.com/go-logr/logr"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes/fake"
"k8s.io/client-go/rest"
"sigs.k8s.io/controller-runtime/pkg/envtest"
"sigs.k8s.io/controller-runtime/pkg/log/zap"
)

var (
restConfig *rest.Config
err error
env *envtest.Environment
)

func TestMain(m *testing.M) {
restConfig, env, err = testutils.StartTestingEnv()
if err != nil {
panic(err)
}

code := m.Run()
testutils.CleanupTestingEnv(env)
os.Exit(code)
}

func TestLogger(t *testing.T) {
t.Run("logs are json structured", func(t *testing.T) {
logOut := new(bytes.Buffer)
Expand Down Expand Up @@ -96,9 +75,3 @@ func TestGetSelfDeploy(t *testing.T) {
require.Nil(t, self)
})
}

func TestNewManagerForRestConfig(t *testing.T) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

can't test things that use the utils.upsert function with the fake environment. That's fine. NewManagerForRestConfig is essentially the main function and doesn't have any logic by itself. Isn't valuable to unit test in the first place.

This will be covered by all e2e tests as well.

conf := &config.Config{NS: "app-routing-system", OperatorDeployment: "operator-test", MetricsAddr: "0"}
_, err := NewManagerForRestConfig(conf, restConfig)
require.NoError(t, err)
}
59 changes: 59 additions & 0 deletions pkg/controller/crd.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
package controller

import (
"context"
"fmt"
"os"
"path/filepath"

"github.com/Azure/aks-app-routing-operator/pkg/config"
"github.com/Azure/aks-app-routing-operator/pkg/util"
"github.com/go-logr/logr"
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/yaml"
)

// loadCRDs loads the CRDs from the specified path into the cluster
func loadCRDs(c client.Client, cfg *config.Config, log logr.Logger) error {
if cfg == nil {
return fmt.Errorf("config cannot be nil")
}

log = log.WithValues("crdPath", cfg.CrdPath)
log.Info("reading crd directory")
files, err := os.ReadDir(cfg.CrdPath)
if err != nil {
return fmt.Errorf("reading crd directory %s: %w", cfg.CrdPath, err)
}

log.Info("applying crds")
for _, file := range files {
if file.IsDir() {
continue
}

path := filepath.Join(cfg.CrdPath, file.Name())
log := log.WithValues("path", path)
log.Info("reading crd file")
var content []byte
content, err := os.ReadFile(path)
if err != nil {
return fmt.Errorf("reading crd file %s: %w", path, err)
}

log.Info("unmarshalling crd file")
crd := &apiextensionsv1.CustomResourceDefinition{}
if err := yaml.Unmarshal(content, crd); err != nil {
return fmt.Errorf("unmarshalling crd file %s: %w", path, err)
}

log.Info("upserting crd")
if err := util.Upsert(context.Background(), c, crd); err != nil {
return fmt.Errorf("upserting crd %s: %w", crd.Name, err)
}
}

log.Info("crds loaded")
return nil
}
47 changes: 47 additions & 0 deletions pkg/controller/crd_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package controller

import (
"context"
"strings"
"testing"

"github.com/Azure/aks-app-routing-operator/pkg/config"
"github.com/go-logr/logr"
"github.com/stretchr/testify/require"
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
)

const (
validCrdPath = "../../config/crd/bases/"
validCrdName = "nginxingresscontrollers.approuting.kubernetes.azure.com"

nonCrdManifestsPath = "../manifests/fixtures/nginx"
nonExistentFilePath = "./this/does/not/exist"
)

func TestLoadCRDs(t *testing.T) {
t.Run("valid crds", func(t *testing.T) {
cl := fake.NewClientBuilder().WithScheme(scheme).Build()
require.NoError(t, loadCRDs(cl, &config.Config{CrdPath: validCrdPath}, logr.Discard()), "expected no error loading valid crds")

crd := &apiextensionsv1.CustomResourceDefinition{}
crd.Name = validCrdName
require.NoError(t, cl.Get(context.Background(), client.ObjectKeyFromObject(crd), crd, nil), "getting loaded valid crd")
})

t.Run("invalid crds", func(t *testing.T) {
cl := fake.NewClientBuilder().WithScheme(scheme).Build()
err := loadCRDs(cl, &config.Config{CrdPath: nonCrdManifestsPath}, logr.Discard())
require.Error(t, err, "expected error loading invalid crds")
require.True(t, strings.Contains(err.Error(), "unmarshalling crd file"), "expected error to be about umarshalling crd")
})

t.Run("non-existent crd path", func(t *testing.T) {
cl := fake.NewClientBuilder().WithScheme(scheme).Build()
err := loadCRDs(cl, &config.Config{CrdPath: nonExistentFilePath}, logr.Discard())
require.Error(t, err, "expected error loading non-existent crd path")
require.True(t, strings.Contains(err.Error(), "reading crd directory"), "expected error to be about reading crd directory")
})
}