Skip to content

Commit

Permalink
Remove config-sync dependency from bootstrap controller (#431)
Browse files Browse the repository at this point in the history 
Remove config-sync lookup from bootstrap controller.
Minor refactoring.
Add unit test coverage.
  • Loading branch information
@efiacor
efiacor authored Dec 1, 2023
1 parent 1c3aa4d commit 096eb99
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 88 deletions.
154 changes: 68 additions & 86 deletions controllers/pkg/reconcilers/bootstrap-packages/reconciler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ import (
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/yaml"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand All @@ -49,12 +48,6 @@ func init() {
reconcilerinterface.Register("bootstrappackages", &reconciler{})
}

const (
stagingNameKey = "nephio.org/staging"
clusterNameKey = "nephio.org/cluster-name"
configsyncNamespace = "config-management-system"
)

//+kubebuilder:rbac:groups="*",resources=secrets,verbs=get;list;watch
//+kubebuilder:rbac:groups=cluster.x-k8s.io,resources=clusters,verbs=get;list;watch
//+kubebuilder:rbac:groups=cluster.x-k8s.io,resources=clusters/status,verbs=get
Expand Down Expand Up @@ -93,7 +86,7 @@ type reconciler struct {
func (r *reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
log := log.FromContext(ctx)
cr := &porchv1alpha1.PackageRevision{}
if err := r.Get(ctx, req.NamespacedName, cr); err != nil {
if err := r.Client.Get(ctx, req.NamespacedName, cr); err != nil {
// There's no need to requeue if we no longer exist. Otherwise we'll be
// requeued implicitly because we return an error.
if resource.IgnoreNotFound(err) != nil {
Expand All @@ -106,84 +99,62 @@ func (r *reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu

// check if the packagerevision is part of a staging repository
// if not we can ignore this package revision
stagingPR, err := r.IsStagingPackageRevision(ctx, cr)
stagingPR, err := r.IsStagingPackageRevision(ctx, cr.Spec.RepositoryName)
if err != nil {
msg := "cannot list repositories"
log.Error(err, msg)
return ctrl.Result{}, errors.Wrap(err, msg)
}
if stagingPR && porchv1alpha1.LifecycleIsPublished(cr.Spec.Lifecycle) {
log.Info("reconcile package revision")
resources, namespacePresent, err := r.getResources(ctx, req)
// get the relevant package revision resources
resources, err := r.getPrResources(ctx, req)
if err != nil {
msg := "cannot get resources"
log.Error(err, msg)
return ctrl.Result{}, errors.Wrap(err, msg)
}
// we expect the clusterName to be applied to all resources in the
// package revision resources, so we find the cluster name by looking at the
// first resource in the resource list
if len(resources) > 0 {
clusterName, ok := resources[0].GetAnnotations()[clusterNameKey]
// we expect the clusterName to be applied to all resources in the
// package revision resources, so we find the cluster name by looking at the
// first resource in the resource list
clusterName, ok := resources[0].GetAnnotations()["nephio.org/cluster-name"]
if !ok {
log.Info("clusterName not found",
"resource", fmt.Sprintf("%s.%s.%s", resources[0].GetAPIVersion(), resources[0].GetKind(), resources[0].GetName()),
"annotations", resources[0].GetAnnotations())
return ctrl.Result{}, nil
}
// we need to find the cluster client
secrets := &corev1.SecretList{}
if err := r.List(ctx, secrets); err != nil {
msg := "cannot list secrets"
clusterSecret, err := r.GetClusterSecret(ctx, clusterName)
if err != nil {
msg := fmt.Sprintf("failed to get cluster Secret for: %s", clusterName)
log.Error(err, msg)
return ctrl.Result{}, errors.Wrap(err, msg)
}
found := false
for _, secret := range secrets.Items {
if strings.Contains(secret.GetName(), clusterName) {
secret := secret // required to prevent gosec warning: G601 (CWE-118): Implicit memory aliasing in for loop
clusterClient, ok := cluster.Cluster{Client: r.Client}.GetClusterClient(&secret)
if ok {
found = true
clusterClient, ready, err := clusterClient.GetClusterClient(ctx)
if err != nil {
msg := "cannot get clusterClient"
log.Error(err, msg)
return ctrl.Result{RequeueAfter: 30 * time.Second}, errors.Wrap(err, msg)
}
if !ready {
log.Info("cluster not ready")
return ctrl.Result{RequeueAfter: 10 * time.Second}, nil
}
if !namespacePresent {
ns := &corev1.Namespace{}
if err = clusterClient.Get(ctx, types.NamespacedName{Name: configsyncNamespace}, ns); err != nil {
if resource.IgnoreNotFound(err) != nil {
msg := fmt.Sprintf("cannot get namespace: %s", configsyncNamespace)
log.Error(err, msg)
return ctrl.Result{RequeueAfter: 30 * time.Second}, errors.Wrap(err, msg)
}
msg := fmt.Sprintf("namespace: %s, does not exist, retry...", configsyncNamespace)
log.Info(msg)
return ctrl.Result{RequeueAfter: 10 * time.Second}, nil
}
}
// install resources
for _, resource := range resources {
resource := resource // required to prevent gosec warning: G601 (CWE-118): Implicit memory aliasing in for loop
log.Info("install manifest", "resource",
fmt.Sprintf("%s.%s.%s", resource.GetAPIVersion(), resource.GetKind(), resource.GetName()))
if err := clusterClient.Apply(ctx, &resource); err != nil {
msg := fmt.Sprintf("cannot apply resource to cluster: resourceName: %s", resource.GetName())
log.Error(err, msg)
return ctrl.Result{}, errors.Wrap(err, msg)
}
}
clusterClient, ok := cluster.Cluster{Client: r.Client}.GetClusterClient(&clusterSecret)
if ok {
clusterClient, ready, err := clusterClient.GetClusterClient(ctx)
if err != nil {
msg := "cannot get clusterClient"
log.Error(err, msg)
return ctrl.Result{RequeueAfter: 30 * time.Second}, errors.Wrap(err, msg)
}
if !ready {
log.Info("cluster not ready")
return ctrl.Result{RequeueAfter: 10 * time.Second}, nil
}
// install the resources to the cluster
for _, resource := range resources {
resource := resource // required to prevent gosec warning: G601 (CWE-118): Implicit memory aliasing in for loop
log.Info("install manifest", "resource", fmt.Sprintf("%s.%s.%s", resource.GetAPIVersion(), resource.GetKind(), resource.GetName()))
if err := clusterClient.Apply(ctx, &resource); err != nil {
msg := fmt.Sprintf("cannot apply resource to cluster: resourceName: %s", resource.GetName())
log.Error(err, msg)
return ctrl.Result{}, errors.Wrap(err, msg)
}
}
}
if !found {
// the cluster client was not found, we retry
} else {
// the clusterClient was not found, we retry
log.Info("cluster client not found, retry...")
return ctrl.Result{RequeueAfter: 10 * time.Second}, nil
}
Expand All @@ -192,38 +163,52 @@ func (r *reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
return ctrl.Result{}, nil
}

func (r *reconciler) IsStagingPackageRevision(ctx context.Context, cr *porchv1alpha1.PackageRevision) (bool, error) {
func (r *reconciler) GetClusterSecret(ctx context.Context, clusterName string) (corev1.Secret, error) {
// we need to find the cluster client secret
secrets := &corev1.SecretList{}
if err := r.Client.List(ctx, secrets); err != nil {
return corev1.Secret{}, err
}
clusterSecret := corev1.Secret{}
for _, secret := range secrets.Items {
if strings.Contains(secret.GetName(), clusterName) {
clusterSecret = secret
break
}
}
return clusterSecret, nil
}

func (r *reconciler) IsStagingPackageRevision(ctx context.Context, repositoryName string) (bool, error) {
repos := &porchconfigv1alpha1.RepositoryList{}
if err := r.porchClient.List(ctx, repos); err != nil {

return false, err
}

stagingRepoNames := []string{}
for _, repo := range repos.Items {
if _, ok := repo.Annotations[stagingNameKey]; ok {
if _, ok := repo.Annotations["nephio.org/staging"]; ok {
stagingRepoNames = append(stagingRepoNames, repo.GetName())
}
}
for _, stagingRepoName := range stagingRepoNames {
if cr.Spec.RepositoryName == stagingRepoName {
if repositoryName == stagingRepoName {
return true, nil
}
}
return false, nil
}

func (r *reconciler) getResources(ctx context.Context, req ctrl.Request) ([]unstructured.Unstructured, bool, error) {
prr := &porchv1alpha1.PackageRevisionResources{}
if err := r.porchClient.Get(ctx, req.NamespacedName, prr); err != nil {
func (r *reconciler) getPrResources(ctx context.Context, req ctrl.Request) ([]unstructured.Unstructured, error) {
PackageRevisionResources := &porchv1alpha1.PackageRevisionResources{}
if err := r.porchClient.Get(ctx, req.NamespacedName, PackageRevisionResources); err != nil {
log.FromContext(ctx).Error(err, "cannot get package revision resourcelist", "key", req.NamespacedName)
return nil, false, err
return nil, err
}

return r.getResourcesPRR(ctx, prr.Spec.Resources)
return r.filterNonLocalResources(ctx, PackageRevisionResources.Spec.Resources)
}

func includeFile(path string, match []string) bool {
func includedFileTypes(path string, match []string) bool {
for _, m := range match {
file := filepath.Base(path)
if matched, err := filepath.Match(m, file); err == nil && matched {
Expand All @@ -233,10 +218,10 @@ func includeFile(path string, match []string) bool {
return false
}

func (r *reconciler) getResourcesPRR(ctx context.Context, resources map[string]string) ([]unstructured.Unstructured, bool, error) {
func (r *reconciler) filterNonLocalResources(ctx context.Context, resources map[string]string) ([]unstructured.Unstructured, error) {
inputs := []kio.Reader{}
for path, data := range resources {
if includeFile(path, []string{"*.yaml", "*.yml", "Kptfile"}) {
if includedFileTypes(path, []string{"*.yaml", "*.yml", "Kptfile"}) {
inputs = append(inputs, &kio.ByteReader{
Reader: strings.NewReader(data),
SetAnnotations: map[string]string{
Expand All @@ -253,25 +238,22 @@ func (r *reconciler) getResourcesPRR(ctx context.Context, resources map[string]s
Outputs: []kio.Writer{&pb},
}.Execute()
if err != nil {
return nil, false, err
return nil, err
}

namespacepresent := false
ul := []unstructured.Unstructured{}
for _, n := range pb.Nodes {
if v, ok := n.GetAnnotations()[filters.LocalConfigAnnotation]; ok && v == "true" {
for _, rnode := range pb.Nodes {
// filter out resources with the following annotation "config.kubernetes.io/local-config"
if v, ok := rnode.GetAnnotations()[filters.LocalConfigAnnotation]; ok && v == "true" {
continue
}
u := unstructured.Unstructured{}
if err := yaml.Unmarshal([]byte(n.MustString()), &u); err != nil {
log.FromContext(ctx).Error(err, "cannot unmarshal data", "data", n.MustString())
// we don't fail
if err := yaml.Unmarshal([]byte(rnode.MustString()), &u); err != nil {
log.FromContext(ctx).Error(err, "cannot unmarshal data", "data", rnode.MustString())
// we dont fail
continue
}
if u.GetKind() == reflect.TypeOf(corev1.Namespace{}).Name() {
namespacepresent = true
}
ul = append(ul, u)
}
return ul, namespacepresent, nil
return ul, nil
}
3 changes: 1 addition & 2 deletions controllers/pkg/reconcilers/bootstrap-packages/reconciler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"context"
"fmt"
"testing"

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

Expand Down Expand Up @@ -76,7 +75,7 @@ spec:
for name, tc := range cases {
t.Run(name, func(t *testing.T) {
r := reconciler{}
us, _, err := r.getResourcesPRR(context.Background(), tc.resources)
us, err := r.filterNonLocalResources(context.Background(), tc.resources)

if tc.expectedErr {
assert.Error(t, err)
Expand Down
1 change: 1 addition & 0 deletions default-go-test.mk
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.


GO_VERSION ?= 1.20.2
TEST_COVERAGE_FILE=lcov.info
TEST_COVERAGE_HTML_FILE=coverage_unit.html
Expand Down

0 comments on commit 096eb99

Please sign in to comment.