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 Deployment resource struct. #2847

Merged
merged 10 commits into from
Sep 10, 2019

Conversation

tejal29
Copy link
Member

@tejal29 tejal29 commented Sep 10, 2019

Add Deployment resource struct and use that instead of using map[string]Deadline for tracking resources.
Pass the same object to print summary.
The summary will now print the namespace of the object

Output change:
Before: skaffold run --default-repo gcr.io/tejal-test --status-check --namespac=test

skaffold run --default-repo gcr.io/tejal-test --status-check --namespac=test
Generating tags...
...
Deploy complete in 3.513243437s
Waiting for deployments to stabilize
 - deployment/leeroy-web is ready. [1/2 deployment(s) still pending]
 - deployment/leeroy-app is ready.
Deployments stabilized in 1.150739535s

After:

tejaldesai@@microservices (small_resource)$ skaffold run --default-repo gcr.io/tejal-test --status-check --namespace=test
Generating tags...
...
Deploy complete in 3.513243437s
Waiting for deployments to stabilize
 - test:deployment/leeroy-app is ready. [1/2 deployment(s) still pending]
 - test:deployment/leeroy-web is ready.
Deployments stabilized in 1.150739535s

}

func (d *Deployment) String() string {
return fmt.Sprintf("%s:%s/%s", d.namespace, d.rType, d.name)
Copy link
Member Author

Choose a reason for hiding this comment

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

This will print test:deployment/leeroy-app where test is namespace and leeroy-app is deployment name

@codecov
Copy link

codecov bot commented Sep 10, 2019

Codecov Report

Merging #2847 into master will increase coverage by 0.04%.
The diff coverage is 75%.

Impacted Files Coverage Δ
pkg/skaffold/deploy/resource/deployment.go 100% <100%> (ø)
pkg/skaffold/deploy/status_check.go 62.22% <60%> (+0.42%) ⬆️

Copy link
Contributor

@balopat balopat left a comment

Choose a reason for hiding this comment

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

LGTM

@balopat
Copy link
Contributor

balopat commented Sep 10, 2019

  • integration test TestRunUnstableChecked is failing as now the name of the deployment has namespace + deployment and we are checking that string - maybe we should relax that check to be less fragile

  • you have some lints

@tejal29 tejal29 merged commit ea013b3 into GoogleContainerTools:master Sep 10, 2019
@tejal29 tejal29 deleted the small_resource branch September 18, 2019 23:05
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.

3 participants