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

app manifest should support referencing a docker image with docker registry #696

Closed
jbayer opened this issue Nov 25, 2015 · 14 comments
Closed

Comments

@jbayer
Copy link

jbayer commented Nov 25, 2015

i didn't see support for it here and assume that it means there is no support for specifying a docker image reference in a cf app manifest.yml

cli/cf/manifest/manifest.go

Lines 167 to 223 in aa1c389

func mapToAppParams(basePath string, yamlMap generic.Map) (models.AppParams, error) {
err := checkForNulls(yamlMap)
if err != nil {
return models.AppParams{}, err
}
var appParams models.AppParams
var errs []error
appParams.BuildpackUrl = stringValOrDefault(yamlMap, "buildpack", &errs)
appParams.DiskQuota = bytesVal(yamlMap, "disk_quota", &errs)
domainAry := *sliceOrEmptyVal(yamlMap, "domains", &errs)
if domain := stringVal(yamlMap, "domain", &errs); domain != nil {
domainAry = append(domainAry, *domain)
}
appParams.Domains = removeDuplicatedValue(domainAry)
hostsArr := *sliceOrEmptyVal(yamlMap, "hosts", &errs)
if host := stringVal(yamlMap, "host", &errs); host != nil {
hostsArr = append(hostsArr, *host)
}
appParams.Hosts = removeDuplicatedValue(hostsArr)
appParams.Name = stringVal(yamlMap, "name", &errs)
appParams.Path = stringVal(yamlMap, "path", &errs)
appParams.StackName = stringVal(yamlMap, "stack", &errs)
appParams.Command = stringValOrDefault(yamlMap, "command", &errs)
appParams.Memory = bytesVal(yamlMap, "memory", &errs)
appParams.InstanceCount = intVal(yamlMap, "instances", &errs)
appParams.HealthCheckTimeout = intVal(yamlMap, "timeout", &errs)
appParams.NoRoute = boolVal(yamlMap, "no-route", &errs)
appParams.NoHostname = boolVal(yamlMap, "no-hostname", &errs)
appParams.UseRandomHostname = boolVal(yamlMap, "random-route", &errs)
appParams.ServicesToBind = sliceOrEmptyVal(yamlMap, "services", &errs)
appParams.EnvironmentVars = envVarOrEmptyMap(yamlMap, &errs)
appParams.HealthCheckType = stringVal(yamlMap, "health-check-type", &errs)
if appParams.Path != nil {
path := *appParams.Path
if filepath.IsAbs(path) {
path = filepath.Clean(path)
} else {
path = filepath.Join(basePath, path)
}
appParams.Path = &path
}
if len(errs) > 0 {
message := ""
for _, err := range errs {
message = message + fmt.Sprintf("%s\n", err.Error())
}
return models.AppParams{}, errors.New(message)
}
return appParams, nil
}

@cf-gitbot
Copy link

We have created an issue in Pivotal Tracker to manage this. You can view the current status of your issue at: https://www.pivotaltracker.com/story/show/108983470.

@dkoper
Copy link

dkoper commented Feb 13, 2016

Hi @jbayer,

Correct. We are focusing on exposing back-end features such as the routing related ones and manifest support is falling behind. It hasn't fallen off my radar though!

Regards,
Dies Koper
CF CLI PM

@dkoper
Copy link

dkoper commented Jul 6, 2016

@SocalNick The docker upload API is still marked experimental. What are the plans with it?
http://apidocs.cloudfoundry.org/238/apps/creating_a_docker_app_%28experimental%29.html

@jbayer
Copy link
Author

jbayer commented Jul 6, 2016

i'm not aware of reasons it should still marked experimental. are there any?

/cc @dieucao

@SocalNick
Copy link

Maybe because docker_credentials_json and ports are still experimental? We can remove the experimental tag from the top-level if you like. Story: https://www.pivotaltracker.com/story/show/126058395

FWIW, we intend to remove docker_credentials_json as part of this story until we officially tackle private docker repos: https://www.pivotaltracker.com/story/show/92603918

@rohitgkk
Copy link

rohitgkk commented Jan 16, 2017

Hi @SocalNick and @dkoper
there is a manifest.yml option docker_image: which can be used to push cloud foundry image but I was unable to do so.

My question is can we push docker image via manifest? ( docker_image: )

@jbayer
Copy link
Author

jbayer commented Jul 7, 2017

@dkoper @dieucao @zrob now that there is proper private docker repository support in CF, not having a docker image manifest option is one of the last things needed to make the docker image experience 1st class. is support for this planned?

@dkoper
Copy link

dkoper commented Jul 7, 2017

@jbayer Definitely!

@wmnnd
Copy link

wmnnd commented Jul 7, 2017

@dkoper Great to hear that Docker support is now planned for manifests. Is there an ETA when we can expect this feature?

@kindrowboat
Copy link

👍

@dkoper
Copy link

dkoper commented Jul 13, 2017

@wmnnd, @motevets Are you planning to use only public Docker images, or also from private Docker repositories?

The story for Docker image support in manifests is currently scheduled for delivery in 1-2 months.
Private Docker repo support in manifest files may take longer: we need to explore a solution where we preferably not force people to include secrets in their app manifest.

The ETA for the former is based on the current refactor of push (and subsequent refactor of create-app-manifest): current push has a lot of code and not written well to be easily extended. We're cleaning it up (and adding lots of tests) to make it easier to enhance, but that takes time.

If demand is high enough, we can at least investigate how much effort and risk is involved in implementing it in the current push ahead of the refactor completion...

@wmnnd
Copy link

wmnnd commented Jul 13, 2017

@dkoper I am using only private registries. For now, I am using cf curl /v2/apps with a JSON file that looks something like a manifest already and then a shell script to attach the necessary services and routes:

{
  "name": "myapp",
  "memory": 128,
  "space_guid": "",
  "docker_image": "my.registry/image",
  "docker_credentials": {
    "username": "foo",
    "password": "bar"
  },
  "environment_json": {
    "FOO": "bar"
  },
  "diego": true
}

If I may make a suggestion about avoiding putting the credentials for private registries in the manifest: You already have CF_DOCKER_PASSWORD, so if you added CF_DOCKER_USER, I think all would be good 😄

@kindrowboat
Copy link

kindrowboat commented Jul 13, 2017 via email

@dkoper
Copy link

dkoper commented Sep 16, 2017

This feature is now supported in cf CLI 6.31.0 onwards.
For now the username can be set in the app manifest and the password passed in with an environment variable. We are exploring other ways to provide both in the future.

@dkoper dkoper closed this as completed Sep 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants