From 9d00824c73e9be98be361188916c9bcde3789f6d Mon Sep 17 00:00:00 2001 From: Manabu Mccloskey Date: Fri, 24 Nov 2023 17:35:59 +0000 Subject: [PATCH] address review comments Signed-off-by: Manabu Mccloskey --- api/v1alpha1/gitrepository_types.go | 4 +-- api/v1alpha1/localbuild_types.go | 20 ++++++++---- api/v1alpha1/zz_generated.deepcopy.go | 32 +++++++++++++++++++ pkg/controllers/gitrepository/controller.go | 3 +- .../gitrepository/controller_test.go | 6 ++-- .../gitrepository/{ => test}/resources/file1 | 0 pkg/controllers/localbuild/argo.go | 2 +- pkg/controllers/localbuild/controller.go | 4 +-- pkg/controllers/localbuild/nginx.go | 2 +- .../idpbuilder.cnoe.io_gitrepositories.yaml | 6 ++-- .../idpbuilder.cnoe.io_localbuilds.yaml | 22 +++++++------ 11 files changed, 71 insertions(+), 30 deletions(-) rename pkg/controllers/gitrepository/{ => test}/resources/file1 (100%) diff --git a/api/v1alpha1/gitrepository_types.go b/api/v1alpha1/gitrepository_types.go index 6820fd38..2146316c 100644 --- a/api/v1alpha1/gitrepository_types.go +++ b/api/v1alpha1/gitrepository_types.go @@ -47,9 +47,9 @@ type GitRepositoryStatus struct { // ExternalGitRepositoryUrl is the url for the in-cluster repository accessible from local machine. // +kubebuilder:validation:Optional ExternalGitRepositoryUrl string `json:"externalGitRepositoryUrl"` - // GitRepositoryUrl is the url for the in-cluster repository accessible within the cluster. + // InternalGitRepositoryUrl is the url for the in-cluster repository accessible within the cluster. // +kubebuilder:validation:Optional - GitRepositoryUrl string `json:"gitRepositoryUrl"` + InternalGitRepositoryUrl string `json:"internalGitRepositoryUrl"` // Path is the path within the repository that contains the files. // +kubebuilder:validation:Optional Path string `json:"path"` diff --git a/api/v1alpha1/localbuild_types.go b/api/v1alpha1/localbuild_types.go index 67e24bc1..f76169ea 100644 --- a/api/v1alpha1/localbuild_types.go +++ b/api/v1alpha1/localbuild_types.go @@ -39,13 +39,10 @@ type LocalbuildSpec struct { type LocalbuildStatus struct { // ObservedGeneration is the 'Generation' of the Service that was last processed by the controller. // +optional - ObservedGeneration int64 `json:"observedGeneration,omitempty"` - - GitServerAvailable bool `json:"gitServerAvailable,omitempty"` - ArgoAvailable bool `json:"argoAvailable,omitempty"` - NginxAvailable bool `json:"nginxAvailable,omitempty"` - ArgoAppsCreated bool `json:"argoAppsCreated,omitempty"` - Gitea GiteaStatus `json:"giteaStatus,omitempty"` + ObservedGeneration int64 `json:"observedGeneration,omitempty"` + ArgoCD ArgoCDStatus `json:"ArgoCD,omitempty"` + Nginx NginxStatus `json:"nginx,omitempty"` + Gitea GiteaStatus `json:"gitea,omitempty"` } type GiteaStatus struct { @@ -56,6 +53,15 @@ type GiteaStatus struct { AdminUserSecretNamespace string `json:"adminUserSecretNamespace,omitempty"` } +type ArgoCDStatus struct { + Available bool `json:"available,omitempty"` + AppsCreated bool `json:"appsCreated,omitempty"` +} + +type NginxStatus struct { + Available bool `json:"available,omitempty"` +} + // +kubebuilder:object:root=true // +kubebuilder:subresource:status // +kubebuilder:resource:path=localbuilds,scope=Cluster diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 2f6f8bc2..6399c8b1 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -25,6 +25,21 @@ import ( runtime "k8s.io/apimachinery/pkg/runtime" ) +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ArgoCDStatus) DeepCopyInto(out *ArgoCDStatus) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ArgoCDStatus. +func (in *ArgoCDStatus) DeepCopy() *ArgoCDStatus { + if in == nil { + return nil + } + out := new(ArgoCDStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ArgoPackageConfigSpec) DeepCopyInto(out *ArgoPackageConfigSpec) { *out = *in @@ -390,6 +405,8 @@ func (in *LocalbuildSpec) DeepCopy() *LocalbuildSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *LocalbuildStatus) DeepCopyInto(out *LocalbuildStatus) { *out = *in + out.ArgoCD = in.ArgoCD + out.Nginx = in.Nginx out.Gitea = in.Gitea } @@ -403,6 +420,21 @@ func (in *LocalbuildStatus) DeepCopy() *LocalbuildStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *NginxStatus) DeepCopyInto(out *NginxStatus) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NginxStatus. +func (in *NginxStatus) DeepCopy() *NginxStatus { + if in == nil { + return nil + } + out := new(NginxStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PackageConfigsSpec) DeepCopyInto(out *PackageConfigsSpec) { *out = *in diff --git a/pkg/controllers/gitrepository/controller.go b/pkg/controllers/gitrepository/controller.go index 9e36be9d..b2d5a751 100644 --- a/pkg/controllers/gitrepository/controller.go +++ b/pkg/controllers/gitrepository/controller.go @@ -30,7 +30,7 @@ const ( giteaAdminPasswordKey = "password" requeueTime = time.Second * 30 gitCommitAuthorName = "git-reconciler" - gitCommitAuthorEmail = "invalid@cnoe.io" + gitCommitAuthorEmail = "idpbuilder-agent@cnoe.io" ) type GiteaClientFunc func(url string, options ...gitea.ClientOption) (GiteaClient, error) @@ -235,7 +235,6 @@ func reconcileRepo(giteaClient GiteaClient, repo *v1alpha1.GitRepository) (*gite if CErr != nil { return &gitea.Repository{}, fmt.Errorf("failed to create git repository: %w", CErr) } - repo.Status.ExternalGitRepositoryUrl = createResp.CloneURL return createResp, nil } } diff --git a/pkg/controllers/gitrepository/controller_test.go b/pkg/controllers/gitrepository/controller_test.go index e2555a7b..78c73e7c 100644 --- a/pkg/controllers/gitrepository/controller_test.go +++ b/pkg/controllers/gitrepository/controller_test.go @@ -96,7 +96,7 @@ func setUpLocalRepo() (string, string, error) { fileObject.SetType(plumbing.BlobObject) w, _ := fileObject.Writer() - file, err := os.ReadFile("resources/file1") + file, err := os.ReadFile("test/resources/file1") if err != nil { return "", "", fmt.Errorf("reading file from resources dir: %w", err) } @@ -146,7 +146,7 @@ func setupDir() (string, error) { return "", fmt.Errorf("creating temporary directory: %w", err) } - file, err := os.ReadFile("resources/file1") + file, err := os.ReadFile("test/resources/file1") if err != nil { return "", fmt.Errorf("reading file from resources dir: %w", err) } @@ -283,7 +283,7 @@ func TestGitRepositoryReconcile(t *testing.T) { if err != nil { t.Fatalf("failed setting up local git repo: %v", err) } - resourcePath, err := filepath.Abs("./resources") + resourcePath, err := filepath.Abs("./test/resources") if err != nil { t.Fatalf("failed to get absolute path: %v", err) } diff --git a/pkg/controllers/gitrepository/resources/file1 b/pkg/controllers/gitrepository/test/resources/file1 similarity index 100% rename from pkg/controllers/gitrepository/resources/file1 rename to pkg/controllers/gitrepository/test/resources/file1 diff --git a/pkg/controllers/localbuild/argo.go b/pkg/controllers/localbuild/argo.go index eb0f68fa..8da63361 100644 --- a/pkg/controllers/localbuild/argo.go +++ b/pkg/controllers/localbuild/argo.go @@ -51,6 +51,6 @@ func (r *LocalbuildReconciler) ReconcileArgo(ctx context.Context, req ctrl.Reque return result, err } - resource.Status.ArgoAvailable = true + resource.Status.ArgoCD.Available = true return ctrl.Result{}, nil } diff --git a/pkg/controllers/localbuild/controller.go b/pkg/controllers/localbuild/controller.go index c87f3601..04a2047a 100644 --- a/pkg/controllers/localbuild/controller.go +++ b/pkg/controllers/localbuild/controller.go @@ -141,7 +141,7 @@ func (r *LocalbuildReconciler) ReconcileEmbeddedGitServer(ctx context.Context, r log := log.FromContext(ctx) // Bail if argo is not yet available - if !resource.Status.ArgoAvailable { + if !resource.Status.ArgoCD.Available { log.Info("argo not yet available, not installing embedded git server") return ctrl.Result{}, nil } @@ -265,7 +265,7 @@ func (r *LocalbuildReconciler) ReconcileArgoAppsWithGitServer(ctx context.Contex } } - resource.Status.ArgoAppsCreated = true + resource.Status.ArgoCD.AppsCreated = true r.shouldShutdown = true return ctrl.Result{}, nil diff --git a/pkg/controllers/localbuild/nginx.go b/pkg/controllers/localbuild/nginx.go index 96701f99..74848ad2 100644 --- a/pkg/controllers/localbuild/nginx.go +++ b/pkg/controllers/localbuild/nginx.go @@ -40,6 +40,6 @@ func (r *LocalbuildReconciler) ReconcileNginx(ctx context.Context, req ctrl.Requ return result, err } - resource.Status.NginxAvailable = true + resource.Status.Nginx.Available = true return ctrl.Result{}, nil } diff --git a/pkg/controllers/resources/idpbuilder.cnoe.io_gitrepositories.yaml b/pkg/controllers/resources/idpbuilder.cnoe.io_gitrepositories.yaml index 380ea889..22b3ec88 100644 --- a/pkg/controllers/resources/idpbuilder.cnoe.io_gitrepositories.yaml +++ b/pkg/controllers/resources/idpbuilder.cnoe.io_gitrepositories.yaml @@ -89,9 +89,9 @@ spec: description: ExternalGitRepositoryUrl is the url for the in-cluster repository accessible from local machine. type: string - gitRepositoryUrl: - description: GitRepositoryUrl is the url for the in-cluster repository - accessible within the cluster. + internalGitRepositoryUrl: + description: InternalGitRepositoryUrl is the url for the in-cluster + repository accessible within the cluster. type: string path: description: Path is the path within the repository that contains diff --git a/pkg/controllers/resources/idpbuilder.cnoe.io_localbuilds.yaml b/pkg/controllers/resources/idpbuilder.cnoe.io_localbuilds.yaml index 1723d4f8..a426d676 100644 --- a/pkg/controllers/resources/idpbuilder.cnoe.io_localbuilds.yaml +++ b/pkg/controllers/resources/idpbuilder.cnoe.io_localbuilds.yaml @@ -64,13 +64,14 @@ spec: type: object status: properties: - argoAppsCreated: - type: boolean - argoAvailable: - type: boolean - gitServerAvailable: - type: boolean - giteaStatus: + ArgoCD: + properties: + appsCreated: + type: boolean + available: + type: boolean + type: object + gitea: properties: adminUserSecretNameecret: type: string @@ -83,8 +84,11 @@ spec: internalURL: type: string type: object - nginxAvailable: - type: boolean + nginx: + properties: + available: + type: boolean + type: object observedGeneration: description: ObservedGeneration is the 'Generation' of the Service that was last processed by the controller.