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

Add spec autoCreateNamespace configuration for creating a ns when doesn't exist #4504

Merged
merged 36 commits into from
Jul 24, 2023

Conversation

hungran
Copy link
Contributor

@hungran hungran commented Jul 15, 2023

What this PR does / why we need it:

Which issue(s) this PR fixes: #1202

Fixes #

Does this PR introduce a user-facing change?: N/A

  • How are users affected by this change: ? not sure
  • Is this breaking change: N/A
  • How to migrate (if breaking change): N/A

@hungran hungran force-pushed the hungran/autoCreateNamespace branch from 7d9c089 to f68861e Compare July 15, 2023 12:27
@khanhtc1202 khanhtc1202 changed the title feat: add spec input.AutoCreateNamespace for creating a ns when doesn't exist Add spec autoCreateNamespace configuration for creating a ns when doesn't exist Jul 20, 2023
pkg/config/application_kubernetes_test.go Outdated Show resolved Hide resolved
pkg/config/application_test.go Outdated Show resolved Hide resolved
pkg/config/application_test.go Outdated Show resolved Hide resolved
pkg/config/application_test.go Outdated Show resolved Hide resolved
pkg/config/application_test.go Outdated Show resolved Hide resolved
pkg/config/application_test.go Outdated Show resolved Hide resolved
hungran and others added 18 commits July 21, 2023 11:56
Signed-off-by: hungran <[email protected]>
Signed-off-by: hungran <[email protected]>

Signed-off-by: hungran <[email protected]>
* Redirect /docs/ to /docs-{latest}/

Signed-off-by: Kenta Kozuka <[email protected]>

* Fix use local path

Signed-off-by: Kenta Kozuka <[email protected]>

---------

Signed-off-by: Kenta Kozuka <[email protected]>
Signed-off-by: hungran <[email protected]>
Co-authored-by: Khanh Tran <[email protected]>
Signed-off-by: hungran <[email protected]>
Co-authored-by: Khanh Tran <[email protected]>
Signed-off-by: hungran <[email protected]>
Bumps [word-wrap](https://github.com/jonschlinkert/word-wrap) from 1.2.3 to 1.2.4.
- [Release notes](https://github.com/jonschlinkert/word-wrap/releases)
- [Commits](jonschlinkert/word-wrap@1.2.3...1.2.4)

---
updated-dependencies:
- dependency-name: word-wrap
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: hungran <[email protected]>
Signed-off-by: khanhtc1202 <[email protected]>
Signed-off-by: hungran <[email protected]>
Signed-off-by: seitarof <[email protected]>
Co-authored-by: Khanh Tran <[email protected]>
Signed-off-by: hungran <[email protected]>
* Update go test command

Signed-off-by: khanhtc1202 <[email protected]>

* Update makefile and github action command

Signed-off-by: khanhtc1202 <[email protected]>

* Revert gitignore

Signed-off-by: khanhtc1202 <[email protected]>

---------

Signed-off-by: khanhtc1202 <[email protected]>
Signed-off-by: hungran <[email protected]>
Signed-off-by: khanhtc1202 <[email protected]>
Signed-off-by: hungran <[email protected]>
* Add Google Tag Manager to the site

Signed-off-by: Kenta Kozuka <[email protected]>

* Add breakline

Signed-off-by: Kenta Kozuka <[email protected]>

* Add breakline

Signed-off-by: Kenta Kozuka <[email protected]>

---------

Signed-off-by: Kenta Kozuka <[email protected]>
Signed-off-by: hungran <[email protected]>
@@ -36,3 +37,4 @@ spec:
helmValueFiles:
- values.yaml
helmVersion: 3.1.1
autoCreateNamespace: false
Copy link
Member

Choose a reason for hiding this comment

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

Default is false so no need to add this

@@ -613,7 +618,8 @@ func TestGenericAnalysisConfiguration(t *testing.T) {
},
},
Input: KubernetesDeploymentInput{
AutoRollback: newBoolPointer(true),
AutoRollback: newBoolPointer(true),
AutoCreateNamespace: false,
Copy link
Member

Choose a reason for hiding this comment

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

Default is false so no need to add this

@@ -493,7 +497,8 @@ func TestGenericPostSyncConfiguration(t *testing.T) {
},
},
Input: KubernetesDeploymentInput{
AutoRollback: newBoolPointer(true),
AutoRollback: newBoolPointer(true),
AutoCreateNamespace: false,
Copy link
Member

Choose a reason for hiding this comment

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

Default is false so no need to add this

@@ -395,7 +397,8 @@ func TestTrueByDefaultBoolConfiguration(t *testing.T) {
},
},
Input: KubernetesDeploymentInput{
AutoRollback: newBoolPointer(false),
AutoRollback: newBoolPointer(false),
AutoCreateNamespace: false,
Copy link
Member

Choose a reason for hiding this comment

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

Default is false so no need to add this

@@ -315,7 +315,8 @@ func TestGenericTriggerConfiguration(t *testing.T) {
},
},
Input: KubernetesDeploymentInput{
AutoRollback: newBoolPointer(true),
AutoRollback: newBoolPointer(true),
AutoCreateNamespace: false,
Copy link
Member

Choose a reason for hiding this comment

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

Default is false so no need to add this

@@ -366,7 +367,8 @@ func TestTrueByDefaultBoolConfiguration(t *testing.T) {
},
},
Input: KubernetesDeploymentInput{
AutoRollback: newBoolPointer(true),
AutoRollback: newBoolPointer(true),
AutoCreateNamespace: false,
Copy link
Member

Choose a reason for hiding this comment

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

Default is false so no need to add this

@@ -97,7 +97,8 @@ func TestKubernetesApplicationConfig(t *testing.T) {
},
},
Input: KubernetesDeploymentInput{
AutoRollback: newBoolPointer(true),
AutoRollback: newBoolPointer(true),
AutoCreateNamespace: false,
Copy link
Member

Choose a reason for hiding this comment

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

Default is false so no need to add this

@@ -135,7 +136,8 @@ func TestKubernetesApplicationConfig(t *testing.T) {
},
},
Input: KubernetesDeploymentInput{
AutoRollback: newBoolPointer(true),
AutoRollback: newBoolPointer(true),
AutoCreateNamespace: false,
Copy link
Member

Choose a reason for hiding this comment

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

Default is false so no need to add this

a.platformProvider.KubeConfigPath,
a.getNamespaceToRun(manifest.Key),
)
if err != nil && !errors.Is(err, errReplaceAlreadyExists) {
Copy link
Member

Choose a reason for hiding this comment

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

Rename due to #4504 (comment)

a.platformProvider.KubeConfigPath,
a.getNamespaceToRun(manifest.Key),
)
if err != nil && !errors.Is(err, errReplaceAlreadyExists) {
Copy link
Member

Choose a reason for hiding this comment

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

Rename due to #4504 (comment)

Copy link
Member

@khanhtc1202 khanhtc1202 left a comment

Choose a reason for hiding this comment

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

Looks neat, just left a few nits. Thank you ❤️

@codecov
Copy link

codecov bot commented Jul 22, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.05 ⚠️

Comparison is base (59663e4) 30.03% compared to head (3daa32d) 29.99%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4504      +/-   ##
==========================================
- Coverage   30.03%   29.99%   -0.05%     
==========================================
  Files         220      220              
  Lines       25761    25796      +35     
==========================================
  Hits         7737     7737              
- Misses      17379    17414      +35     
  Partials      645      645              
Impacted Files Coverage Δ
...g/app/piped/platformprovider/kubernetes/applier.go 0.00% <0.00%> (ø)
...g/app/piped/platformprovider/kubernetes/kubectl.go 0.00% <0.00%> (ø)
pkg/config/application_kubernetes.go 8.00% <ø> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@@ -81,6 +92,17 @@ func (a *applier) CreateManifest(ctx context.Context, manifest Manifest) error {
return a.initErr
}

if a.input.AutoCreateNamespace == true {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if a.input.AutoCreateNamespace == true {
if a.input.AutoCreateNamespace {

@@ -64,6 +64,17 @@ func (a *applier) ApplyManifest(ctx context.Context, manifest Manifest) error {
return a.initErr
}

if a.input.AutoCreateNamespace == true {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if a.input.AutoCreateNamespace == true {
if a.input.AutoCreateNamespace {

khanhtc1202
khanhtc1202 previously approved these changes Jul 23, 2023
Copy link
Member

@khanhtc1202 khanhtc1202 left a comment

Choose a reason for hiding this comment

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

Great work, thank you ❤️

@khanhtc1202
Copy link
Member

@hungran Could you check the false test 🙏
https://github.com/pipe-cd/pipecd/actions/runs/5634502628/job/15264954333?pr=4504
(On local, you can run make test/go to run the test as on the CI)

@hungran
Copy link
Contributor Author

hungran commented Jul 23, 2023

@hungran Could you check the false test 🙏 https://github.com/pipe-cd/pipecd/actions/runs/5634502628/job/15264954333?pr=4504 (On local, you can run make test/go to run the test as on the CI)

@khanhtc1202 yeah, I might missed run the test after changed the test file, fixed

@hungran hungran requested a review from khanhtc1202 July 23, 2023 06:25
@@ -22,6 +22,7 @@ spec:
helmValueFiles:
- values.yaml
helmVersion: 3.1.1
autoCreateNamespace: true
Copy link
Member

Choose a reason for hiding this comment

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

Do we need this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think no, will remove it :) thanks

@hungran hungran requested a review from khanhtc1202 July 23, 2023 07:19
Copy link
Member

@khanhtc1202 khanhtc1202 left a comment

Choose a reason for hiding this comment

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

Thank you 👍

@khanhtc1202 khanhtc1202 merged commit e51fff9 into pipe-cd:master Jul 24, 2023
11 of 13 checks passed
@github-actions github-actions bot mentioned this pull request Aug 23, 2023
moko-poi pushed a commit to moko-poi/pipecd that referenced this pull request Nov 3, 2023
…sn't exist (pipe-cd#4504)

* feat: add spec input.AutoCreateNamespace for creating a ns when doesn't exist

Signed-off-by: hungran <[email protected]>

* not return error when namespace already exists

Signed-off-by: hungran <[email protected]>
Signed-off-by: hungran <[email protected]>

* not return err

Signed-off-by: hungran <[email protected]>

* WIP
Signed-off-by: hungran <[email protected]>

Signed-off-by: hungran <[email protected]>

* omitempty AutoCreateNamespace, add test case

Signed-off-by: hungran <[email protected]>

* test data k8s-app-helm.yaml

Signed-off-by: hungran <[email protected]>

* Redirect /docs/ to /docs-{latest}/ (pipe-cd#4495)

* Redirect /docs/ to /docs-{latest}/

Signed-off-by: Kenta Kozuka <[email protected]>

* Fix use local path

Signed-off-by: Kenta Kozuka <[email protected]>

---------

Signed-off-by: Kenta Kozuka <[email protected]>
Signed-off-by: hungran <[email protected]>

* Fix documentation link is not displayed (pipe-cd#4505)

Signed-off-by: hungran <[email protected]>

* Update docs/content/en/docs-dev/user-guide/configuration-reference.md

Co-authored-by: Khanh Tran <[email protected]>
Signed-off-by: hungran <[email protected]>

* Update pkg/config/application_kubernetes.go

Co-authored-by: Khanh Tran <[email protected]>
Signed-off-by: hungran <[email protected]>

* Update pkg/config/application_kubernetes_test.go

Co-authored-by: Khanh Tran <[email protected]>
Signed-off-by: hungran <[email protected]>

* Update pkg/config/application_test.go

Co-authored-by: Khanh Tran <[email protected]>
Signed-off-by: hungran <[email protected]>

* Bump word-wrap from 1.2.3 to 1.2.4 in /web (pipe-cd#4511)

Bumps [word-wrap](https://github.com/jonschlinkert/word-wrap) from 1.2.3 to 1.2.4.
- [Release notes](https://github.com/jonschlinkert/word-wrap/releases)
- [Commits](jonschlinkert/word-wrap@1.2.3...1.2.4)

---
updated-dependencies:
- dependency-name: word-wrap
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: hungran <[email protected]>

* Setup codecov (pipe-cd#4509)

Signed-off-by: khanhtc1202 <[email protected]>
Signed-off-by: hungran <[email protected]>

* add: caution note to user group doc (pipe-cd#4512)

Signed-off-by: seitarof <[email protected]>
Co-authored-by: Khanh Tran <[email protected]>
Signed-off-by: hungran <[email protected]>

* Update go test command (pipe-cd#4514)

* Update go test command

Signed-off-by: khanhtc1202 <[email protected]>

* Update makefile and github action command

Signed-off-by: khanhtc1202 <[email protected]>

* Revert gitignore

Signed-off-by: khanhtc1202 <[email protected]>

---------

Signed-off-by: khanhtc1202 <[email protected]>
Signed-off-by: hungran <[email protected]>

* Fix make release/docs command regenerate unused docs directory (pipe-cd#4513)

Signed-off-by: khanhtc1202 <[email protected]>
Signed-off-by: hungran <[email protected]>

* Update auth docs (pipe-cd#4517)

Signed-off-by: khanhtc1202 <[email protected]>
Signed-off-by: hungran <[email protected]>

* Add Google Tag Manager to the site (pipe-cd#4519)

* Add Google Tag Manager to the site

Signed-off-by: Kenta Kozuka <[email protected]>

* Add breakline

Signed-off-by: Kenta Kozuka <[email protected]>

* Add breakline

Signed-off-by: Kenta Kozuka <[email protected]>

---------

Signed-off-by: Kenta Kozuka <[email protected]>
Signed-off-by: hungran <[email protected]>

* Update pkg/config/application_kubernetes_test.go

Signed-off-by: hungran <[email protected]>

* Update pkg/config/application_test.go

Signed-off-by: hungran <[email protected]>

* Update pkg/config/application_test.go

Signed-off-by: hungran <[email protected]>

* Update pkg/config/application_test.go

Signed-off-by: hungran <[email protected]>

* Update pkg/config/application_test.go

Signed-off-by: hungran <[email protected]>

* Update pkg/config/application_test.go

Signed-off-by: hungran <[email protected]>

* get value from var not from pointer

Signed-off-by: hungran <[email protected]>

* handle error when namespace already exsist and ignore it

Signed-off-by: hungran <[email protected]>

* rename handle err, remove unessacarry value test

Signed-off-by: hungran <[email protected]>

* if true

Signed-off-by: hungran <[email protected]>

* fix test data

Signed-off-by: hungran <[email protected]>

* fix correct the test case

Signed-off-by: hungran <[email protected]>

* remove unuse value in k8s-app-helm

Signed-off-by: hungran <[email protected]>

---------

Signed-off-by: hungran <[email protected]>
Signed-off-by: hungran <[email protected]>
Signed-off-by: Kenta Kozuka <[email protected]>
Signed-off-by: dependabot[bot] <[email protected]>
Signed-off-by: khanhtc1202 <[email protected]>
Signed-off-by: seitarof <[email protected]>
Co-authored-by: Kenta Kozuka <[email protected]>
Co-authored-by: Khanh Tran <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Seitaro Fujigaki <[email protected]>
Signed-off-by: moko-poi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants