redhat-developer / odo

odo - Developer-focused CLI for fast & iterative container-based application development on Podman and Kubernetes. Implementation of the open Devfile standard.
https://odo.dev
Apache License 2.0
795 stars 243 forks source link

behavior of `odo create` when `--binary` is a directory #854

Closed tmds closed 4 years ago

tmds commented 6 years ago

I'm trying out odo with .NET Core.

.NET Core applications do not compile to a single file, but a directory that has multiple files. I'm passing the directory as the --binary argument.

odo create dotnet:2.1 myapp --binary ./bin/Release/netcoreapp2.1/publish

This doesn't work as expected. My intent is to upload all files in the publish directory, but the directory itself is also uploaded.

As a workaround, this works:

odo create dotnet:2.1 myapp --local ./bin/Release/netcoreapp2.1/publish

Can the behavior of binary be changed so it matches that of --local when the argument is a directory? Related question: why does odo have different arguments for --local and --binary?

kadel commented 6 years ago

Related question: why does odo have different arguments for --local and --binary?

local says that it is source code that needs to be compiled binary says that it is already compiled and it just needs to be run

binary was originaly added with just Java in mind where you usualy push just jar or war file.

We will have to update binary flag to work with directories. @tmds can you point us to some nice dotnet example that we can play with to make sure that it is working?

tmds commented 6 years ago

local says that it is source code that needs to be compiled binary says that it is already compiled and it just needs to be run

The s2i builder isn't informed. It gets a some content. Based on the content it determines if the source is 'already compiled' or 'source code that needs to be compiled'. So the distinction made by odo is artificial. Maybe splitting the two with separate arguments is simpler for the user, that is a UX question, which I don't have an opinion about :)

@tmds can you point us to some nice dotnet example that we can play with to make sure that it is working?

You need a dotnet sdk. For Fedora, https://developer.fedoraproject.org/tech/languages/csharp/dotnet-installation.html For RHEL/CentOS: https://access.redhat.com/documentation/en-us/net_core/2.1/html/getting_started_guide/gs_install_dotnet#install_dotnet21 For other: www.dot.net/download

To create the publish folder:

$ dotnet new web -o webapp
$ cd webapp
$ dotnet publish -c Release /p:MicrosoftNETPlatformLibrary=Microsoft.NETCore.App

This generates a folder at bin/Release/netcoreapp2.1/publish

surajnarwade commented 6 years ago

Hey @tmds , I am trying to create an app with dotnet, I got this error,

$ dotnet new web -o webapp
Cannot get required symbol TLSv1_1_method from libssl
Aborted (core dumped)

can you please help me with this ?

tmds commented 6 years ago

What OS are you using? What version of OpenSSL do you have? Try installing OpenSSL 1.1 package or OpenSSL 1.0-compat package.

CC @omajid

surajnarwade commented 6 years ago

@tmds , I am using fedora 28

$ openssl version
OpenSSL 1.1.0i-fips  14 Aug 2018
tmds commented 6 years ago

Probably it will work when you install the OpenSSL 1.0 compat package.

omajid commented 6 years ago

@surajnarwade You should dnf install compat-openssl10 on Fedora 28.

tmds commented 6 years ago

@omajid why doesn't that come as a dependency of the dotnet runtime package? Can we add it?

omajid commented 6 years ago

@tmds It does come as a dependency if you use the copr packages:

$ sudo dnf repoquery --requires dotnet-runtime-2.1 --resolve
Last metadata expiration check: 0:32:02 ago on Wed 31 Oct 2018 05:07:11 PM EDT.
compat-openssl10-1:1.0.2o-3.fc29.x86_64
dotnet-host-0:2.1.5-2.fc29.x86_64
glibc-0:2.28-9.fc29.i686
glibc-0:2.28-9.fc29.x86_64
krb5-libs-0:1.16.1-21.fc29.x86_64
libcurl-0:7.61.1-3.fc29.x86_64
libcurl-minimal-0:7.61.1-3.fc29.x86_64
libgcc-0:8.2.1-4.fc29.x86_64
libicu-0:62.1-2.fc29.i686
libicu-0:62.1-2.fc29.x86_64
libstdc++-0:8.2.1-4.fc29.x86_64
libunwind-0:1.2.1-6.fc29.x86_64
lttng-ust-0:2.10.1-4.fc29.x86_64
zlib-0:1.2.11-14.fc29.x86_64
geoand commented 6 years ago

Related question: why does odo have different arguments for --local and --binary?

local says that it is source code that needs to be compiled binary says that it is already compiled and it just needs to be run

binary was originaly added with just Java in mind where you usualy push just jar or war file.

We will have to update binary flag to work with directories. @tmds can you point us to some nice dotnet example that we can play with to make sure that it is working?

@kadel @jorgemoralespou Do we have an initial issue where the design and or differences between --local and --binary were discussed?

jorgemoralespou commented 6 years ago

@geoand No. In fact, initially binary was just for files that have been built locally and local for sources that need to be built in the container, but:

At the end of the day, currently I would think that local handles the directory use case for dotnet if you specify a directory. And binary just works for files. Given the names used for the options to me makes sense how it works. Maybe the problem with dotnet can be fixed with documentation.

geoand commented 6 years ago

@jorgemoralespou Thanks!

As someone you has never used dotnet but that is somewhat familiar with odo, I would expect to just use --local instead of binary, so I agree that the issue could perhaps be addressed with better documentation

tmds commented 6 years ago

I don't understand why the behavior for binary isn't changed when the argument is a folder. It wouldn't break anything. As I said before, I'm not sure why the tool distinguished between local and binary. Maybe we can remove binary? Then there is no confusion on what option to use.

jorgemoralespou commented 6 years ago

Basically we want to keep the notion of when you're pushing something that has already been locally built vs something that needs to be built in the container. This will allow us in the future to support different images than s2i images, which are the only thing we can support ATM.

This means that until this is fixed, you need to use --local ./bin/Release/netcoreapp2.1/publish and will achieve what you expect.

@geoand we should make binary support uploading a directory for the reason above mentioned and not rely on the user setting --local to a directory holding binaries.

Hope this makes sense.

geoand commented 6 years ago

@jorgemoralespou I agree that makes a lot of sense.

I do think however that this support will require a fair amount of more work that what was put into #954 :)

tmds commented 6 years ago

It shouldn't be much work to make binary for a directory use the same implementation as local. That will bring the expected odo UX also for .NET Core developers.

Improving performance for uploading directories can be considered a separate issue (independent of .NET Core).

geoand commented 6 years ago

@tmds Agreed that it shouldn't be too much work, but it will definitely be more than I put into the PR I linked :)

metacosm commented 6 years ago

Maybe --local should be renamed to --source and make --binary work with directories, because --local doesn't really convey much information about what it's about…

jorgemoralespou commented 6 years ago

@metacosm But also --source could be interpreted as can provide any url that accepts source code, which can be a local file system or a remote git repository.

Many terms could be ambiguous and selecting one over the other will be difficult. But in any case, I think it's something we can challenge, whether --local should be changed to something more clearly understood.

@jankleinert @gshipley @marekjelen @ryanj @joshix @grahamdumpleton @mhausenblas any opinion on this?

mhausenblas commented 6 years ago

+1 for --source since it clearly indicates that some source code is involved, be it local or via a (remote) Git URL.

jorgemoralespou commented 6 years ago

@mhausenblas, it would only for local source, hence the question. For git based source, there's --git, as this flag changes the behavior of the interactive development pod.

mhausenblas commented 6 years ago

Sorry for not being clear @jorgemoralespou I meant --source for both local and Git source. KISS.

tmds commented 5 years ago

I'd love to see this in odo for better .NET Core support. Any thoughts when this may be implemented?

girishramnani commented 5 years ago

@kadel @jorgemoralespou is this issue still relevant?

tmds commented 5 years ago

Yes. It's a UX issue when using odo with .NET Core binary builds.

openshift-bot commented 4 years ago

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close. Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

openshift-bot commented 4 years ago

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten. Rotten issues close after an additional 30d of inactivity. Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten /remove-lifecycle stale

openshift-bot commented 4 years ago

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen. Mark the issue as fresh by commenting /remove-lifecycle rotten. Exclude this issue from closing again by commenting /lifecycle frozen.

/close

openshift-ci-robot commented 4 years ago

@openshift-bot: Closing this issue.

In response to [this](https://github.com/openshift/odo/issues/854#issuecomment-636959857): >Rotten issues close after 30d of inactivity. > >Reopen the issue by commenting `/reopen`. >Mark the issue as fresh by commenting `/remove-lifecycle rotten`. >Exclude this issue from closing again by commenting `/lifecycle frozen`. > >/close Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.