[BUG] Validate update calls for setting task resource attributes

[katrogan]

Describe the bug

Setting the task resource attributes for a project with
{"project":"foo","domain":"development","limits":{"cpu":"4096","memory":"8Ti"}}
causes flyteadmin to panic on resolving task resource attributes at create execution time

We shouldn't accept invalid configs or handle partial task resource attributes overrides gracefully

Expected behavior

Admin should not panic at create execution time for invalid or partial task resource attributes

Additional context to reproduce

No response

Screenshots

No response

Are you sure this issue hasn't been raised already?

• Yes

Have you read the Code of Conduct?

• Yes

[hhcs9527]

Hi @katrogan,
I am interested in this topic, but I am still new to flyte.
Can I try this issue?

[katrogan]

hi @hhcs9527 absolutely!
you should be able to repro this on a flyte sandbox installation by updating task resource attributes for one of the boilerplate projects (like flytesnacks) using the resources in the GH issue. Then try launching an execution and verify that it causes admin to panic

This is the code that handles reading the overriden task resource attributes:
https://github.com/flyteorg/flyteadmin/blob/master/pkg/manager/impl/execution_manager.go#L531-L534 at execution time

[github-actions]

Hello 👋, this issue has been inactive for over 9 months. To help maintain a clean and focused backlog, we'll be marking this issue as stale and will engage on it to decide if it is still applicable.
Thank you for your contribution and understanding! 🙏

[asd0300]

#take

[asd0300]

Hi @katrogan
I try update task resource attributes
with yaml

project: flytesnacks
domain: development
limits:
  cpu: "4096"
  memory: "8Ti"
  gpu: "2"

update success
then execute task

like to

@task(requests=Resources(cpu="4096", mem="8Ti", gpu="2"))
def say_hello() -> str:
    return "Hello, World!"

response
Running Execution on Remote. 0:00:00 Running execution on remote. [✔] Go to http://localhost:30080/console/projects/flytesnacks/domains/development/executions/av7jnqfhmg989hz8672l to see execution in the console.

seems can't repro the panic case

[katrogan]

hi @asd0300 thank you for taking a look! what happens if you remove the gpu from the yaml?

[asd0300]

test yaml will be

project: flytesnacks
domain: development
limits:
  cpu: "4096"
  memory: "8Ti"

before update, remove the default task-resource-attribute
then update the test .yaml -> updated success

then run task
1.

@task(requests=Resources(cpu="4096", mem="8Ti", gpu="2"))
def say_hello() -> str:
    return "Hello, World!"

or
2.

@task(requests=Resources(cpu="4096", mem="8Ti"))
def say_hello() -> str:
    return "Hello, World!"

both tasks could be executed with no panic occurred

[katrogan]

thank you for verifying! sounds like this is no longer an issue - let's close it!